NetBSD Problem Report #55102

From www@netbsd.org  Tue Mar 24 03:00:41 2020
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-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 D93CE1A9213
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 24 Mar 2020 03:00:41 +0000 (UTC)
Message-Id: <20200324030040.6C8F71A921F@mollari.NetBSD.org>
Date: Tue, 24 Mar 2020 03:00:40 +0000 (UTC)
From: n54@gmx.com
Reply-To: n54@gmx.com
To: gnats-bugs@NetBSD.org
Subject: ODR violation in fifo_vnodeop_opv_desc
X-Send-Pr-Version: www-1.0

>Number:         55102
>Category:       kern
>Synopsis:       ODR violation in fifo_vnodeop_opv_desc
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Mar 24 03:05:00 +0000 2020
>Closed-Date:    Thu May 14 21:50:23 +0000 2020
>Last-Modified:  Sat May 16 00:45:01 +0000 2020
>Originator:     Kamil Rytarowski
>Release:        NetBSD 9.99.50 amd64
>Organization:
TNF
>Environment:
NetBSD chieftec 9.99.50 NetBSD 9.99.50 (GENERIC) #0: Sat Mar 21 14:11:45 CET 2020  root@chieftec:/public/netbsd-root/sys/arch/amd64/compile/GENERIC amd64
>Description:
fifo_vnodeop_opv_desc is defined twice:

     91 int (**fifo_vnodeop_p)(void *);
     92 const struct vnodeopv_entry_desc fifo_vnodeop_entries[] = {
     93 	{ &vop_default_desc, vn_default_error },
     94 	{ &vop_putpages_desc, genfs_null_putpages },
     95 	{ NULL, NULL }
     96 };
     97 const struct vnodeopv_desc fifo_vnodeop_opv_desc =
     98 	{ &fifo_vnodeop_p, fifo_vnodeop_entries };
     99 

src/sys/rump/librump/rumpvfs/rumpfs.c

    631 int (**fifo_vnodeop_p)(void *);
    632 const struct vnodeopv_entry_desc fifo_vnodeop_entries[] = {
    633 	{ &vop_default_desc, vn_default_error },
    634 	{ &vop_lookup_desc, fifo_lookup },		/* lookup */
    635 	{ &vop_create_desc, genfs_badop },		/* create */
    636 	{ &vop_mknod_desc, genfs_badop },		/* mknod */
    637 	{ &vop_open_desc, fifo_open },			/* open */
    638 	{ &vop_close_desc, fifo_close },		/* close */
    639 	{ &vop_access_desc, genfs_ebadf },		/* access */
    640 	{ &vop_getattr_desc, genfs_ebadf },		/* getattr */
    641 	{ &vop_setattr_desc, genfs_ebadf },		/* setattr */
    642 	{ &vop_read_desc, fifo_read },			/* read */
    643 	{ &vop_write_desc, fifo_write },		/* write */
    644 	{ &vop_fallocate_desc, genfs_eopnotsupp },	/* fallocate */
    645 	{ &vop_fdiscard_desc, genfs_eopnotsupp },	/* fdiscard */
    646 	{ &vop_ioctl_desc, fifo_ioctl },		/* ioctl */
    647 	{ &vop_poll_desc, fifo_poll },			/* poll */
    648 	{ &vop_kqfilter_desc, fifo_kqfilter },		/* kqfilter */
    649 	{ &vop_revoke_desc, genfs_revoke },		/* revoke */
    650 	{ &vop_mmap_desc, genfs_badop },		/* mmap */
    651 	{ &vop_fsync_desc, genfs_nullop },		/* fsync */
    652 	{ &vop_seek_desc, genfs_badop },		/* seek */
    653 	{ &vop_remove_desc, genfs_badop },		/* remove */
    654 	{ &vop_link_desc, genfs_badop },		/* link */
    655 	{ &vop_rename_desc, genfs_badop },		/* rename */
    656 	{ &vop_mkdir_desc, genfs_badop },		/* mkdir */
    657 	{ &vop_rmdir_desc, genfs_badop },		/* rmdir */
    658 	{ &vop_symlink_desc, genfs_badop },		/* symlink */
    659 	{ &vop_readdir_desc, genfs_badop },		/* readdir */
    660 	{ &vop_readlink_desc, genfs_badop },		/* readlink */
    661 	{ &vop_abortop_desc, genfs_badop },		/* abortop */
    662 	{ &vop_inactive_desc, fifo_inactive },		/* inactive */
    663 	{ &vop_reclaim_desc, genfs_nullop },		/* reclaim */
    664 	{ &vop_lock_desc, genfs_lock },			/* lock */
    665 	{ &vop_unlock_desc, genfs_unlock },		/* unlock */
    666 	{ &vop_bmap_desc, fifo_bmap },			/* bmap */
    667 	{ &vop_strategy_desc, genfs_badop },		/* strategy */
    668 	{ &vop_print_desc, fifo_print },		/* print */
    669 	{ &vop_islocked_desc, genfs_islocked },		/* islocked */
    670 	{ &vop_pathconf_desc, fifo_pathconf },		/* pathconf */
    671 	{ &vop_advlock_desc, genfs_einval },		/* advlock */
    672 	{ &vop_bwrite_desc, genfs_nullop },		/* bwrite */
    673 	{ &vop_putpages_desc, genfs_null_putpages },	/* putpages */
    674 	{ NULL, NULL }
    675 };
    676 const struct vnodeopv_desc fifo_vnodeop_opv_desc =
    677 	{ &fifo_vnodeop_p, fifo_vnodeop_entries };

src/sys/miscfs/fifofs/fifo_vnops.c

"The One Definition Rule (ODR) is an important rule of the C++ programming language that prescribes that objects and non-inline functions cannot have more than one definition in the entire program and template and types cannot have more than one definition by translation unit. It is defined in the ISO C++ Standard (ISO/IEC 14882) 2003, at section 3.2."

Reported with ASan + RUMPKERNEL:

=================================================================
==9899==ERROR: AddressSanitizer: odr-violation (0x7f7ff6a07c80):
  [1] size=16 'fifo_vnodeop_opv_desc' /usr/src/lib/librumpvfs/../../sys/rump/librump/rumpvfs/rumpfs.c:97:28
  [2] size=16 'fifo_vnodeop_opv_desc' /usr/src/sys/rump/fs/lib/libfifovfs/../../../../miscfs/fifofs/fifo_vnops.c:676:28
These globals were registered at these points:
  [1]:
    #0 0x30cf1e in __asan_register_globals (/usr/tests/fs/ffs/h_ffs_server+0x10cf1e)
    #1 0x7f7ff52c8e7b  (/usr/lib/librumpvfs.so.0+0xc8e7b)

  [2]:
    #0 0x30cf1e in __asan_register_globals (/usr/tests/fs/ffs/h_ffs_server+0x10cf1e)
    #1 0x7f7ff6805e8b  (/usr/lib/librumpvfs_fifofs.so.0+0x5e8b)

==9899==HINT: if you don't care about these errors you may set ASAN_OPTIONS=detect_odr_violation=0
SUMMARY: AddressSanitizer: odr-violation: global 'fifo_vnodeop_opv_desc' at /usr/src/lib/librumpvfs/../../sys/rump/librump/rumpvfs/rumpfs.c:97:28
==9899==ABORTING

If we set detect_odr_violation=1, we get more errors:

=================================================================
==9535==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7f7ff6a07970 at pc 0x7f7ff52bc4be bp 0x7f7fffffe200 sp 0x7f7fffffe1f8
READ of size 8 at 0x7f7ff6a07970 thread T0
    #0 0x7f7ff52bc4bd in vfs_opv_init_explicit /usr/src/lib/librumpvfs/../../sys/rump/../kern/vfs_init.c:206:19
    #1 0x7f7ff52bc4bd in rumpns_vfs_opv_init /usr/src/lib/librumpvfs/../../sys/rump/../kern/vfs_init.c:282:3
    #2 0x7f7ff52bc896 in rumpns_vfsinit /usr/src/lib/librumpvfs/../../sys/rump/../kern/vfs_init.c:430:2
    #3 0x7f7ff52cebce in rumpcompinitRUMP__FACTION_VFS /usr/src/lib/librumpvfs/../../sys/rump/librump/rumpvfs/rump_vfs.c:126:2
    #4 0x7f7ff4e1d1ca in rump_component_init /usr/src/lib/librump/../../sys/rump/librump/rumpkern/rump.c:604:4
    #5 0x7f7ff4e1c551 in rump_init /usr/src/lib/librump/../../sys/rump/librump/rumpkern/rump.c:432:2
    #6 0x3152bc in main /usr/src/tests/fs/ffs/h_ffs_server.c:87:10
    #7 0x21eebc in ___start (/usr/tests/fs/ffs/h_ffs_server+0x1eebc)

0x7f7ff6a07970 is located 0 bytes to the right of global variable 'fifo_vnodeop_entries' defined in '/usr/src/lib/librumpvfs/../../sys/rump/librump/rumpvfs/rumpfs.c:92:34' (0x7f7ff6a07940) of size 48
0x7f7ff6a07970 is located 48 bytes inside of global variable 'fifo_vnodeop_entries' defined in '/usr/src/sys/rump/fs/lib/libfifovfs/../../../../miscfs/fifofs/fifo_vnops.c:632:34' (0x7f7ff6a07940) of size 672
SUMMARY: AddressSanitizer: global-buffer-overflow /usr/src/lib/librumpvfs/../../sys/rump/../kern/vfs_init.c:206:19 in vfs_opv_init_explicit
Shadow bytes around the buggy address:
  0x4feffed40ed0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x4feffed40ee0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x4feffed40ef0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x4feffed40f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x4feffed40f10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x4feffed40f20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00[f9]f9
  0x4feffed40f30: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x4feffed40f40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x4feffed40f50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x4feffed40f60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x4feffed40f70: 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==9535==ABORTING
>How-To-Repeat:
./build.sh -U -V MKDEBUG=yes -V MKDEBUGLIB=yes -V MAKECONF=/dev/null -V
MKLLVMRT=no -V MKX11=no -V MKLLVM=yes -V HAVE_LLVM=yes -V MKGCC=no -V
MKSANITIZER=yes -N0 -j8 -u -O /public/netbsd-llvm/ distribution

mount -t null /dev /path/to/destdir/dev
mount -t null /dev/pts /path/to/destdir/dev/pts
mount -t null /tmp /path/to/destdir/tmp
chroot /path/to/destdir/
cd /usr/tests/fs
export ASAN_OPTIONS=detect_container_overflow=0:log_path=/tmp/atf.zz
atf-run

Check /tmp/atf.zz.* for ASan reports.

>Fix:
N/A

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55102 CVS commit: src/sys/rump/librump/rumpvfs
Date: Thu, 14 May 2020 16:10:34 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Thu May 14 20:10:34 UTC 2020

 Modified Files:
 	src/sys/rump/librump/rumpvfs: rumpfs.c

 Log Message:
 PR/55102: Kamil Rytarowski: Remove fifo stuff duplicated in fifo_vnops.c


 To generate a diff of this commit:
 cvs rdiff -u -r1.158 -r1.159 src/sys/rump/librump/rumpvfs/rumpfs.c

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

State-Changed-From-To: open->closed
State-Changed-By: kamil@NetBSD.org
State-Changed-When: Thu, 14 May 2020 23:50:23 +0200
State-Changed-Why:
Fixed in rumpfs.c 1.159.


From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55102 CVS commit: src
Date: Fri, 15 May 2020 19:32:29 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Fri May 15 23:32:28 UTC 2020

 Modified Files:
 	src/distrib/sets/lists/base: shl.mi
 	src/distrib/sets/lists/comp: mi shl.mi
 	src/distrib/sets/lists/debug: mi shl.mi
 	src/sys/rump/fs: Makefile.rumpfscomp
 	src/sys/rump/librump/rumpvfs: rumpfs.c
 	src/tests: Makefile.inc
 	src/tests/fs/ffs: Makefile
 	src/tests/fs/puffs: Makefile

 Log Message:
 PR/55102: Kamil Rytarowski: Duplicate fifo_vnodeop_entries,
 fifo_vnodeop_opv_desc symbols.

 Many filesystems ffs, lfs, ulfs, chfs, ext2fs etc. use fifofs
 internally for their fifo vnops. NFS does too, but it also needs
 networking anyway.  Unfortunately fifofs brings in a lot of the
 networking code so that the rumpkernel is not well partition. In
 addition the fifo code is rarely used.

 The existing hack depended on duplicating the above symbols and
 adding minimal functionality for the majority of the the tests
 (except the ffs and the puffs one). In these two cases both symbols
 were loaded and the symbol sizes clashed which broke the sanitizers.
 While this can be fixed with weak symbols and other kinds of
 indirection, it is more straight forward to select between the
 minimal and the full fifofs implementation by introducing a new
 shared library librumpvfs_nofifofs.


 To generate a diff of this commit:
 cvs rdiff -u -r1.885 -r1.886 src/distrib/sets/lists/base/shl.mi
 cvs rdiff -u -r1.2325 -r1.2326 src/distrib/sets/lists/comp/mi
 cvs rdiff -u -r1.331 -r1.332 src/distrib/sets/lists/comp/shl.mi
 cvs rdiff -u -r1.309 -r1.310 src/distrib/sets/lists/debug/mi
 cvs rdiff -u -r1.246 -r1.247 src/distrib/sets/lists/debug/shl.mi
 cvs rdiff -u -r1.7 -r1.8 src/sys/rump/fs/Makefile.rumpfscomp
 cvs rdiff -u -r1.160 -r1.161 src/sys/rump/librump/rumpvfs/rumpfs.c
 cvs rdiff -u -r1.6 -r1.7 src/tests/Makefile.inc
 cvs rdiff -u -r1.21 -r1.22 src/tests/fs/ffs/Makefile
 cvs rdiff -u -r1.15 -r1.16 src/tests/fs/puffs/Makefile

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

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55102 CVS commit: src/sys/rump/fs/lib/libnofifovfs
Date: Fri, 15 May 2020 20:43:15 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Sat May 16 00:43:15 UTC 2020

 Added Files:
 	src/sys/rump/fs/lib/libnofifovfs: Makefile nofifo_vnops.c

 Log Message:
 PR/55102: Kamil Rytarowski: Duplicate fifo_vnodeop_entries,
 fifo_vnodeop_opv_desc symbols.

 Missed adding those in previous


 To generate a diff of this commit:
 cvs rdiff -u -r0 -r1.1 src/sys/rump/fs/lib/libnofifovfs/Makefile \
     src/sys/rump/fs/lib/libnofifovfs/nofifo_vnops.c

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

>Unformatted:

Home	PR Database Search