NetBSD Problem Report #40634

From bad@bsd.de  Sat Feb 14 00:39:33 2009
Return-Path: <bad@bsd.de>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 2CDF863C17C
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 14 Feb 2009 00:39:33 +0000 (UTC)
Message-Id: <20090214003929.1AFD71FA7F@just-passing-through.k.bsd.de>
Date: Sat, 14 Feb 2009 01:39:29 +0100 (CET)
From: bad@bsd.de
Reply-To: bad@bsd.de
To: gnats-bugs@gnats.NetBSD.org
Subject: dostatvfs() broken when chroot to /
X-Send-Pr-Version: 3.95

>Number:         40634
>Category:       kern
>Synopsis:       "chroot / /sbin/mount" shows only / as mounted
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Feb 14 00:40:01 +0000 2009
>Closed-Date:    Sat Feb 14 16:55:55 +0000 2009
>Last-Modified:  Sat Feb 14 17:00:03 +0000 2009
>Originator:     Christoph Badura
>Release:        NetBSD 5.99.4
>Organization:
>Environment:
System: NetBSD not-invented-here 5.99.4 NetBSD 5.99.4 (gaol_domu) #29: Sat Feb 14 00:42:09 CET 2009 bad@not-invented-here:/home/bad/gaol_domu i386
Architecture: i386
Machine: i386
>Description:
	When chrooted to / mount and df show only the root FS:

	# chroot / /sbin/mount
	/dev/xbd0a on / type ffs (local)
	#

	This is because the logic in vfs_syscalls.c:dostatvfs() to
	hide mount points outside the chroot fails when the chroot dir is
	/.

>How-To-Repeat:
	chroot / /sbin/mount
>Fix:
Apply the following change to dostatvfs():

 		 * data.
 		 */
 		if (strncmp(bp, sp->f_mntonname, len) == 0 &&
-		    ((c = sp->f_mntonname[len]) == '/' || c == '\0')) {
+		    (len <= 1 || (c = sp->f_mntonname[len]) == '/' || c == '\0')) {
 			(void)strlcpy(sp->f_mntonname, &sp->f_mntonname[len],
 			    sizeof(sp->f_mntonname));
 			if (sp->f_mntonname[0] == '\0')

when the chroot dir is / len is 1 and bp is "/" f_mnttonname[1] is uneqal
to '/' and '\0' for all mount points other than /.

This broke in rev. 1.364 while fixing PR/38745.

The fix should be pulled up to 5.0.

>Release-Note:

>Audit-Trail:
From: enami tsugutomo <enami@sm.sony.co.jp>
To: bad@bsd.de
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/40634: dostatvfs() broken when chroot to /
Date: Sat, 14 Feb 2009 15:43:04 +0900 (JST)

 > >Fix:
 > Apply the following change to dostatvfs():
 > 
 >  		 * data.
 >  		 */
 >  		if (strncmp(bp, sp->f_mntonname, len) == 0 &&
 > -		    ((c = sp->f_mntonname[len]) == '/' || c == '\0')) {
 > +		    (len <= 1 || (c = sp->f_mntonname[len]) == '/' || c == '\0')) {
 >  			(void)strlcpy(sp->f_mntonname, &sp->f_mntonname[len],
 >  			    sizeof(sp->f_mntonname));
 >  			if (sp->f_mntonname[0] == '\0')
 > 
 > when the chroot dir is / len is 1 and bp is "/" f_mnttonname[1] is uneqal
 > to '/' and '\0' for all mount points other than /.

 Your change removes / in front of mount point, doesn't it?

 enami.

 Index: vfs_syscalls.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/vfs_syscalls.c,v
 retrieving revision 1.385
 diff -u -r1.385 vfs_syscalls.c
 --- vfs_syscalls.c	5 Feb 2009 13:37:24 -0000	1.385
 +++ vfs_syscalls.c	14 Feb 2009 06:38:43 -0000
 @@ -888,7 +888,9 @@
  		 * rest we cannot see, so we don't allow viewing the
  		 * data.
  		 */
 -		if (strncmp(bp, sp->f_mntonname, len) == 0 &&
 +		if (len == 1)
 +			/* our root is / */;
 +		else if (strncmp(bp, sp->f_mntonname, len) == 0 &&
  		    ((c = sp->f_mntonname[len]) == '/' || c == '\0')) {
  			(void)strlcpy(sp->f_mntonname, &sp->f_mntonname[len],
  			    sizeof(sp->f_mntonname));

From: Christoph Badura <bad@bsd.de>
To: gnats-bugs@NetBSD.org
Cc: enami tsugutomo <enami@sm.sony.co.jp>
Subject: Re: kern/40634: dostatvfs() broken when chroot to /
Date: Sat, 14 Feb 2009 12:06:32 +0000

 On Sat, Feb 14, 2009 at 06:45:02AM +0000, enami tsugutomo wrote:
 >  Your change removes / in front of mount point, doesn't it?

 Duh!

 I didn't want to lose the comparison of the prefix.

 How about this:

 -		if (strncmp(bp, sp->f_mntonname, len) == 0 &&
 +		if (len == 1 && *bp == '/')
 +			;	/* our root is /, show all mountpoints */
 +		else if (strncmp(bp, sp->f_mntonname, len) == 0 &&

 I also missed the ; hidden after the comment, on first reading.

 --chris

State-Changed-From-To: open->closed
State-Changed-By: christos@NetBSD.org
State-Changed-When: Sat, 14 Feb 2009 11:55:55 -0500
State-Changed-Why:
fixed, thanks


From: Christos Zoulas <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/40634 CVS commit: src/sys/kern
Date: Sat, 14 Feb 2009 16:55:25 +0000 (UTC)

 Module Name:	src
 Committed By:	christos
 Date:		Sat Feb 14 16:55:25 UTC 2009

 Modified Files:
 	src/sys/kern: vfs_syscalls.c

 Log Message:
 PR/40634: Christoph Badura: "chroot / /sbin/mount" shows only / as mounted


 To generate a diff of this commit:
 cvs rdiff -r1.385 -r1.386 src/sys/kern/vfs_syscalls.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