NetBSD Problem Report #29898

From yamt@mwd.biglobe.ne.jp  Wed Apr  6 04:29:03 2005
Return-Path: <yamt@mwd.biglobe.ne.jp>
Received: from yamt.dyndns.org (FLA1Aah135.kng.mesh.ad.jp [61.193.101.135])
	by narn.netbsd.org (Postfix) with ESMTP id 7E54263B121
	for <gnats-bugs@gnats.NetBSD.org>; Wed,  6 Apr 2005 04:29:03 +0000 (UTC)
Message-Id: <1112761737.676810.4731.nullmailer@yamt.dyndns.org>
Date: Wed, 06 Apr 2005 13:28:57 +0900
From: yamt@mwd.biglobe.ne.jp
Reply-To: yamt@mwd.biglobe.ne.jp
To: gnats-bugs@netbsd.org
Subject: mount(2) can corrupt filesystem
X-Send-Pr-Version: 3.95

>Number:         29898
>Category:       kern
>Synopsis:       mount(2) can corrupt filesystem
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Apr 06 04:30:00 +0000 2005
>Last-Modified:  Wed Apr 13 21:32:00 +0000 2005
>Originator:     YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
>Release:        NetBSD 3.99.3
>Organization:

>Environment:


System: NetBSD kaeru 3.99.3 NetBSD 3.99.3 (build.kaeru.nodebug) #35: Tue Apr 5 15:52:06 JST 2005 takashi@kaeru:/home/takashi/work/kernel/build.kaeru.nodebug i386
Architecture: i386
Machine: i386
>Description:
	as mount(2) temporarily alters important mnt_flag like MNT_SOFTDEP,
	if there're other activities on the filesystem in the mean time,
	you'll lose.  in the worst case, it ends up corrupting the filesystem.
	mount(2) with MNT_GETARGS is esp. harmful because it can be done
	by arbitrary users.
>How-To-Repeat:
	code inspection.
>Fix:
	restructure mount related code so that the caller of
	VFS_MOUNT doesn't need to alter mnt_flag/iflag.
	maybe by adding more arguments to VFS_MOUNT to describe the operation.

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: kern-bug-people->yamt
Responsible-Changed-By: yamt@netbsd.org
Responsible-Changed-When: Wed, 06 Apr 2005 11:06:41 +0000
Responsible-Changed-Why:
i'll fix.


From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org,
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc: 
Subject: Re: kern/29898: mount(2) can corrupt filesystem
Date: Wed, 6 Apr 2005 09:32:07 -0400

 On Apr 6,  4:30am, yamt@mwd.biglobe.ne.jp (yamt@mwd.biglobe.ne.jp) wrote:
 -- Subject: kern/29898: mount(2) can corrupt filesystem

 | 	restructure mount related code so that the caller of
 | 	VFS_MOUNT doesn't need to alter mnt_flag/iflag.
 | 	maybe by adding more arguments to VFS_MOUNT to describe the operation.

 Isn't is as simple as doing:

 Index: vfs_syscalls.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/vfs_syscalls.c,v
 retrieving revision 1.217
 diff -u -u -r1.217 vfs_syscalls.c
 --- vfs_syscalls.c	26 Feb 2005 21:34:56 -0000	1.217
 +++ vfs_syscalls.c	6 Apr 2005 13:30:55 -0000
 @@ -216,7 +216,11 @@
  			vput(vp);
  			return (EPERM);
  		}
 -		goto update;
 +		if (SCARGS(uap, flags) & MNT_GETARGS)
 +			goto getargs;
 +		else
 +			goto update;
 +
  	} else {
  		if (securelevel >= 2) {
  			vput(vp);
 @@ -328,6 +332,7 @@
  	    MNT_SYNCHRONOUS | MNT_UNION | MNT_ASYNC | MNT_NOCOREDUMP |
  	    MNT_NOATIME | MNT_NODEVMTIME | MNT_SYMPERM | MNT_SOFTDEP |
  	    MNT_IGNORE);
 + getargs:
  	/*
  	 * Mount the filesystem.
  	 */

From: YAMAMOTO Takashi <yamt@netbsd.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: PR/29898 CVS commit: src/sys/kern
Date: Wed,  6 Apr 2005 13:49:31 +0000 (UTC)

 Module Name:	src
 Committed By:	yamt
 Date:		Wed Apr  6 13:49:31 UTC 2005

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

 Log Message:
 sys_mount:
 - reject attempts of MNT_GETARGS + other MNT_xxx.
 - don't modify mnt_flags needlessly for MNT_GETARGS.
   a stopgap fix for PR/29898.


 To generate a diff of this commit:
 cvs rdiff -r1.217 -r1.218 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.

From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
To: christos@zoulas.com
Cc: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org,
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/29898: mount(2) can corrupt filesystem
Date: Wed, 06 Apr 2005 23:23:20 +0900

 hi,

 > | 	restructure mount related code so that the caller of
 > | 	VFS_MOUNT doesn't need to alter mnt_flag/iflag.
 > | 	maybe by adding more arguments to VFS_MOUNT to describe the operation.
 > 
 > Isn't is as simple as doing:

 well, while i've committed the similar change,
 i don't think it's that simple.  it isn't MNT_GETARGS specific.

 eg. consider updating !MNT_SOFTDEP to MNT_SOFTDEP.
 until ffs_mount notices the attempt and prevents it by setting MNT_SOFTDEP,
 MNT_SOFTDEP is left cleared.

 i think either of the following is needed.
 - change VFS_MOUNT as i suggested above.
 - don't share mnt_flag between filesystem independent code and
   filesystem code.

 YAMAMOTO Takashi

From: christos@zoulas.com (Christos Zoulas)
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
Cc: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org,
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/29898: mount(2) can corrupt filesystem
Date: Wed, 6 Apr 2005 10:31:58 -0400

 On Apr 6, 11:23pm, yamt@mwd.biglobe.ne.jp (YAMAMOTO Takashi) wrote:
 -- Subject: Re: kern/29898: mount(2) can corrupt filesystem

 | hi,
 | 
 | > | 	restructure mount related code so that the caller of
 | > | 	VFS_MOUNT doesn't need to alter mnt_flag/iflag.
 | > | 	maybe by adding more arguments to VFS_MOUNT to describe the operation.
 | > 
 | > Isn't is as simple as doing:
 | 
 | well, while i've committed the similar change,
 | i don't think it's that simple.  it isn't MNT_GETARGS specific.
 | 
 | eg. consider updating !MNT_SOFTDEP to MNT_SOFTDEP.
 | until ffs_mount notices the attempt and prevents it by setting MNT_SOFTDEP,
 | MNT_SOFTDEP is left cleared.
 | 
 | i think either of the following is needed.
 | - change VFS_MOUNT as i suggested above.

 This is too intrusive.

 | - don't share mnt_flag between filesystem independent code and
 |   filesystem code.

 I think that this is better because then the filesystem specific
 code can veto the flag settings before applying them.

 christos

Responsible-Changed-From-To: yamt->kern-bug-people
Responsible-Changed-By: yamt@netbsd.org
Responsible-Changed-When: Thu, 07 Apr 2005 02:04:07 +0000
Responsible-Changed-Why:
there's no consensus about how to fix.


From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
To: christos@zoulas.com
Cc: tech-kern@netbsd.org
Subject: Re: kern/29898: mount(2) can corrupt filesystem
Date: Thu, 07 Apr 2005 11:04:33 +0900

 [ adding Cc: tech-kern@ ]

 > On Apr 6, 11:23pm, yamt@mwd.biglobe.ne.jp (YAMAMOTO Takashi) wrote:
 > -- Subject: Re: kern/29898: mount(2) can corrupt filesystem
 > 
 > | hi,
 > | 
 > | > | 	restructure mount related code so that the caller of
 > | > | 	VFS_MOUNT doesn't need to alter mnt_flag/iflag.
 > | > | 	maybe by adding more arguments to VFS_MOUNT to describe the operation.
 > | > 
 > | > Isn't is as simple as doing:
 > | 
 > | well, while i've committed the similar change,
 > | i don't think it's that simple.  it isn't MNT_GETARGS specific.
 > | 
 > | eg. consider updating !MNT_SOFTDEP to MNT_SOFTDEP.
 > | until ffs_mount notices the attempt and prevents it by setting MNT_SOFTDEP,
 > | MNT_SOFTDEP is left cleared.
 > | 
 > | i think either of the following is needed.
 > | - change VFS_MOUNT as i suggested above.
 > 
 > This is too intrusive.
 > 
 > | - don't share mnt_flag between filesystem independent code and
 > |   filesystem code.
 > 
 > I think that this is better because then the filesystem specific
 > code can veto the flag settings before applying them.
 > 
 > christos

 i have a different opinion.
 it's sometimes better to fix intrusively and it's the case now.
 there's no good reason to have operational flags like MNT_UPDATE in mnt_flag.
 accumulating kludgy fixes is not the way to go.

 YAMAMOTO Takashi

From: Matthias Scheler <tron@netbsd.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: PR/29898 CVS commit: [netbsd-3] src/sys/kern
Date: Wed, 13 Apr 2005 21:31:09 +0000 (UTC)

 Module Name:	src
 Committed By:	tron
 Date:		Wed Apr 13 21:31:09 UTC 2005

 Modified Files:
 	src/sys/kern [netbsd-3]: vfs_syscalls.c

 Log Message:
 Pull up revision 1.218 (requested by yamt in ticket #142):
 sys_mount:
 - reject attempts of MNT_GETARGS + other MNT_xxx.
 - don't modify mnt_flags needlessly for MNT_GETARGS.
   a stopgap fix for PR/29898.


 To generate a diff of this commit:
 cvs rdiff -r1.217 -r1.217.2.1 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