NetBSD Problem Report #56080

From hauke@Espresso.Rhein-Neckar.DE  Sun Mar 28 10:01:20 2021
Return-Path: <hauke@Espresso.Rhein-Neckar.DE>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id C66661A9217
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 28 Mar 2021 10:01:20 +0000 (UTC)
Message-Id: <202103281001.12SA15od011948@pizza.causeuse.org>
Date: Sun, 28 Mar 2021 12:01:05 +0200 (CEST)
From: Hauke Fath <hauke@Espresso.Rhein-Neckar.DE>
Reply-To: Hauke Fath <hauke@Espresso.Rhein-Neckar.DE>
To: gnats-bugs@NetBSD.org
Cc: Hauke Fath <hauke@Espresso.Rhein-Neckar.DE>
Subject: tar(1) dumps core on nfs volume
X-Send-Pr-Version: 3.95

>Number:         56080
>Category:       bin
>Synopsis:       tar(1) dumps core on nfs volume
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lukem
>State:          feedback
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Mar 28 10:05:00 +0000 2021
>Closed-Date:    
>Last-Modified:  Sat Jun 10 16:05:01 +0000 2023
>Originator:     Hauke Fath
>Release:        NetBSD 9.99.81
>Organization:
Falling Raindrops
>Environment:


System: NetBSD i386-pxe 9.99.81 NetBSD 9.99.81 (BLACKBOX-$Revision: 1.85 $) #0: Mon Mar 22 16:48:50 CET 2021  hauke@pizza.causeuse.org:/var/obj/netbsd-builds/developer/amd64/sys/arch/amd64/compile/BLACKBOX amd64
Architecture: x86_64
Machine: amd64

% ldd /bin/tar
/bin/tar:
	-larchive.4 => /usr/lib/libarchive.so.4
	-lbz2.1 => /usr/lib/libbz2.so.1
	-lc.12 => /lib/libc.so.12
	-lcrypto.14 => /lib/libcrypto.so.14
	-lcrypt.1 => /lib/libcrypt.so.1
	-lexpat.2 => /usr/lib/libexpat.so.2
	-llzma.2 => /lib/liblzma.so.2
	-lpthread.1 => /lib/libpthread.so.1
	-lz.1 => /lib/libz.so.1
% ident /bin/tar
/bin/tar:
     $NetBSD: crt0.S,v 1.4 2018/11/26 17:37:46 joerg Exp $
     $NetBSD: crt0-common.c,v 1.23 2018/12/28 20:12:35 christos Exp $
     $NetBSD: crti.S,v 1.1 2010/08/07 18:01:35 joerg Exp $
     $NetBSD: crtbegin.S,v 1.2 2010/11/30 18:37:59 joerg Exp $
     $NetBSD: crtend.S,v 1.1 2010/08/07 18:01:34 joerg Exp $
     $NetBSD: crtn.S,v 1.1 2010/08/07 18:01:35 joerg Exp $
%

>Description:

Building (packaging) editors/xemacs-nox11 on a net-booted -current
installation fails when libarchive-as-tar invoked from
work/xemacs-21.4.24/Makefile dumps core because of an assertion
failure:

[hauke@i386-pxe] /<6>xemacs-21.4.24/lisp > pwd
/var/obj/pkgsrc/editors/xemacs-nox11/work/xemacs-21.4.24/lisp
[hauke@i386-pxe] /<6>xemacs-21.4.24/lisp > tar -cf /dev/null .
Segmentation fault (core dumped)
[hauke@i386-pxe] /<6>xemacs-21.4.24/lisp > gdb /bin/tar tar.core
GNU gdb (GDB) 11.0.50.20200914-git
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64--netbsd".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /bin/tar...
Reading symbols from /usr/libdata/debug//bin/tar.debug...
[New process 6101]
Core was generated by `tar'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00006f8f68467866 in _citrus_iconv_convert (nresults=0x7f7fff388ab8, flags=0, outbytes=0x7f7fff388b38, out=0x7f7fff388b30, 
    inbytes=0x7f7fff388b28, in=0x7f7fff388b20, cv=0x6f8f6a89b0e0) at /usr/src/lib/libc/citrus/citrus_iconv.h:61
61		_DIAGASSERT(cv && cv->cv_shared && cv->cv_shared->ci_ops &&
(gdb) bt
#0  0x00006f8f68467866 in _citrus_iconv_convert (nresults=0x7f7fff388ab8, flags=0, outbytes=0x7f7fff388b38, out=0x7f7fff388b30, 
    inbytes=0x7f7fff388b28, in=0x7f7fff388b20, cv=0x6f8f6a89b0e0) at /usr/src/lib/libc/citrus/citrus_iconv.h:61
#1  _iconv (handle=handle@entry=0x6f8f6a89b0e0, in=in@entry=0x7f7fff388b20, szin=szin@entry=0x7f7fff388b28, out=out@entry=0x7f7fff388b30, 
    szout=szout@entry=0x7f7fff388b38) at /usr/src/lib/libc/iconv/iconv.c:97
#2  0x00006f8f6a483834 in iconv_strncat_in_locale (as=0x6f8f6a832158, _p=<optimized out>, length=<optimized out>, sc=0x6f8f6a883070)
    at /usr/src/external/bsd/libarchive/dist/libarchive/archive_string.c:2039
#3  0x00006f8f6a4844ba in archive_strncat_l (as=as@entry=0x6f8f6a832158, _p=<optimized out>, n=<optimized out>, sc=<optimized out>)
    at /usr/src/external/bsd/libarchive/dist/libarchive/archive_string.c:1998
#4  0x00006f8f6a48459a in archive_strncpy_l (as=as@entry=0x6f8f6a832158, _p=<optimized out>, n=<optimized out>, sc=<optimized out>)
    at /usr/src/external/bsd/libarchive/dist/libarchive/archive_string.c:1944
#5  0x00006f8f6a4847d9 in archive_mstring_get_mbs_l (aes=0x6f8f6a832110, p=0x7f7fff388c58, length=0x7f7fff388c88, sc=<optimized out>)
    at /usr/src/external/bsd/libarchive/dist/libarchive/archive_string.c:4005
#6  0x00006f8f6a47b810 in _archive_entry_pathname_l (entry=<optimized out>, p=<optimized out>, len=<optimized out>, sc=<optimized out>)
    at /usr/src/external/bsd/libarchive/dist/libarchive/archive_entry.c:598
#7  0x00006f8f6a41f051 in get_entry_pathname (a=0x6f8f6a88d000, entry=<optimized out>, name=<optimized out>, length=<optimized out>, 
    sc=<optimized out>) at /usr/src/external/bsd/libarchive/dist/libarchive/archive_write_set_format_pax.c:428
#8  0x00006f8f6a41f5da in archive_write_pax_header (a=0x6f8f6a88d000, entry_original=0x6f8f6a831500)
    at /usr/src/external/bsd/libarchive/dist/libarchive/archive_write_set_format_pax.c:839
#9  0x00006f8f6a46a1da in _archive_write_header (_a=0x6f8f6a88d000, entry=0x6f8f6a831500)
    at /usr/src/external/bsd/libarchive/dist/libarchive/archive_write.c:650
#10 0x00000000e4c08ac2 in write_entry (bsdtar=bsdtar@entry=0x7f7fff389590, a=a@entry=0x6f8f6a88d000, entry=0x6f8f6a831500)
    at /usr/src/external/bsd/libarchive/dist/tar/write.c:974
#11 0x00000000e4c08cac in write_file (entry=<optimized out>, a=0x6f8f6a88d000, bsdtar=0x7f7fff389590)
    at /usr/src/external/bsd/libarchive/dist/tar/write.c:962
#12 write_hierarchy (bsdtar=bsdtar@entry=0x7f7fff389590, a=a@entry=0x6f8f6a88d000, path=path@entry=0x7f7fff389dd2 ".")
    at /usr/src/external/bsd/libarchive/dist/tar/write.c:941
#13 0x00000000e4c0907e in write_archive (a=a@entry=0x6f8f6a88d000, bsdtar=bsdtar@entry=0x7f7fff389590)
    at /usr/src/external/bsd/libarchive/dist/tar/write.c:513
#14 0x00000000e4c098a4 in tar_mode_c (bsdtar=bsdtar@entry=0x7f7fff389590) at /usr/src/external/bsd/libarchive/dist/tar/write.c:247
#15 0x00000000e4c0ba4d in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/external/bsd/libarchive/dist/tar/bsdtar.c:910
(gdb) print *cv
$1 = {cv_shared = 0x6c652e73, cv_closure = 0x6f8f6a80b050}
(gdb) print *cv->cv_shared
Cannot access memory at address 0x6c652e73
(gdb)

where 0x6c652e73 happens to be ascii 's.el'.

See also
<https://mail-index.netbsd.org/current-users/2021/03/26/msg040510.html> ff.


>How-To-Repeat:

	On a net-booted instance of -current amd64, with pkgsrc
	mounted r/o and WRKOBJDIR=/var/obj/pkgsrc, 'make extract'
	editors/xemacs-nox11.

	Change to
	${WRKOBJDIR}/editors/xemacs-nox11/work/xemacs-21.4.24/lisp and
	run '/bin/tar -cf /dev/null .'.

	The assertion failure appears to be somewhat specific to the
	tared up directory.

	/bin/pax works here, as does pkgsrc gtar.


>Fix:

	None, yet. 

	A workaround is to have the xemacs Makefile use GNU tar	instead.



>Release-Note:

>Audit-Trail:
From: RVP <rvp@SDF.ORG>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/56080: tar(1) dumps core on nfs volume
Date: Wed, 31 Mar 2021 21:33:35 +0000 (UTC)

 Fix here:

 https://mail-index.netbsd.org/current-users/2021/03/31/msg040584.html

 -RVP

Responsible-Changed-From-To: bin-bug-people->lukem
Responsible-Changed-By: lukem@NetBSD.org
Responsible-Changed-When: Sun, 28 May 2023 19:52:15 +0000
Responsible-Changed-Why:


State-Changed-From-To: open->analyzed
State-Changed-By: lukem@NetBSD.org
State-Changed-When: Sun, 28 May 2023 19:52:15 +0000
State-Changed-Why:

I read the thread about this issue, including RVP's proposed fix to
ensure that the struct filesystem.name_max field isn't 0 to fallback
to NAME_MAX.

I think libarchive's implementation for NetBSD could be simplified to
just allocate the backing store for sizeof(struct dirent) when using
readdir_r() on platforms where dirent.d_name has a known size (e.g.,
d_name).  Or use the existing logic modified to check at startup to use
NAME_MAX if defined and pathconf as a fallback?

(I see that macOS/Darwin and Linux/glibc both claim readdir_r() is
deprecated because NAME_MAX may be undefined, but NetBSD doesn't claim
that it's deprecated, and AFAICT, neither does POSIX.1-2017).

Looking into more detail in how libarchive uses the filesystem.name_max field:

- Relevant parts of struct filesystem:
        struct filesystem {
        // ...
        #if defined(USE_READDIR_R)
                size_t          name_max;
        #endif
        // ...
        };

- There's a lot of system-specific complexity to determine name_max
  as the length of a path name.

- Relevant parts of struct tree:
        struct tree {
        //...
        #if defined(USE_READDIR_R)
                struct dirent           *dirent;
                size_t                   dirent_allocated;
        #endif
        //...
        };

- In tree_dir_next_posix(), filesystem.name_max is used to determine
  how much space to allocate for struct tree.dirent:
        #if defined(USE_READDIR_R)
                dirent_size = offsetof(struct dirent, d_name) +
                  t->filesystem_table[t->current->filesystem_id].name_max + 1;
                if (t->dirent == NULL || t->dirent_allocated < dirent_size) {
                        free(t->dirent);
                        t->dirent = malloc(dirent_size);


Why does libarchive go to so much effort to determine how much space to
use for the backing store for readdir_r()'s struct dirent in a
system-specific manner?

Why isn't libarchive just allocating sizeof(struct dirent) for those
systems where struct dirent is known to be a fixed size (where
dirent.d_name is fixed), and avoid this complicated
per-current-filesystem statvfs (etc) logic?

It strikes me that this complex logic could potentially fail with memory
overruns if a kernel returns the wrong value for "maximum length of a
path for that filesystem", and given dirent has a fixed upper size at
compile time on most platforms (see above).



From: "Luke Mewburn" <lukem@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56080 CVS commit: src/external/bsd/libarchive/dist/libarchive
Date: Sat, 10 Jun 2023 12:46:32 +0000

 Module Name:	src
 Committed By:	lukem
 Date:		Sat Jun 10 12:46:32 UTC 2023

 Modified Files:
 	src/external/bsd/libarchive/dist/libarchive: archive_read_disk_posix.c

 Log Message:
 libarchive: fail if name_max is 0

 Add error handling to the USE_READDIR_R code paths that set name_max
 from struct statfs or statvfs; if the determined name_max == 0
 then return an error.

 Avoids a crash in tree_dir_next_posix() when the calculation of
 dirent_size from name_max is too small for the memory allocated
 for struct dirent.

 Submitted to upstream in pull request
 	https://github.com/libarchive/libarchive/pull/1903

 Should fix PR bin/56080


 To generate a diff of this commit:
 cvs rdiff -u -r1.2 -r1.3 \
     src/external/bsd/libarchive/dist/libarchive/archive_read_disk_posix.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: analyzed->feedback
State-Changed-By: lukem@NetBSD.org
State-Changed-When: Sat, 10 Jun 2023 16:02:03 +0000
State-Changed-Why:
Does my recent commit to -current fix this crash
(tar should fail with an error instead of crashing)


From: Luke Mewburn <luke@mewburn.net>
To: gnats-bugs@netbsd.org
Cc: lukem@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
	Hauke Fath <hauke@espresso.rhein-neckar.de>
Subject: Re: bin/56080 (tar(1) dumps core on nfs volume)
Date: Sun, 11 Jun 2023 02:00:52 +1000

 I've added error handling in libarchive to error out
 if name_max==0, to avoid crashing.

 I've also submitted a pull request to the libarchive upstream
 with the same fix:
 	https://github.com/libarchive/libarchive/pull/1903

 If upstream revises the fix, I'll backport it here
 (or we'll get when we upgrade to a release that contains it).

>Unformatted:

Home	PR Database Search