From yar@bsd.chem.msu.ru Wed Feb 6 14:32:42 2008 Return-Path: Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D829F16A419 for ; Wed, 6 Feb 2008 14:32:42 +0000 (UTC) (envelope-from yar@bsd.chem.msu.ru) Received: from bsd.chem.msu.ru (bsd.chem.msu.ru [195.208.208.23]) by mx1.freebsd.org (Postfix) with ESMTP id 430EF13C4EC for ; Wed, 6 Feb 2008 14:32:41 +0000 (UTC) (envelope-from yar@bsd.chem.msu.ru) Received: from bsd.chem.msu.ru (localhost [127.0.0.1]) by bsd.chem.msu.ru (8.13.8/8.13.8) with ESMTP id m16EWdUc020742 for ; Wed, 6 Feb 2008 17:32:39 +0300 (MSK) (envelope-from yar@bsd.chem.msu.ru) Received: (from yar@localhost) by bsd.chem.msu.ru (8.13.8/8.13.8/Submit) id m16EWcVO020741; Wed, 6 Feb 2008 17:32:38 +0300 (MSK) (envelope-from yar) Message-Id: <200802061432.m16EWcVO020741@bsd.chem.msu.ru> Date: Wed, 6 Feb 2008 17:32:38 +0300 (MSK) From: Yar Tikhiy Reply-To: Yar Tikhiy To: FreeBSD-gnats-submit@freebsd.org Cc: Subject: fsck on read-only root fs upgrades it to read-write X-Send-Pr-Version: 3.113 X-GNATS-Notify: >Number: 120319 >Category: kern >Synopsis: [patch] fsck on read-only root fs upgrades it to read-write >Confidential: no >Severity: non-critical >Priority: low >Responsible: rodrigc >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Wed Feb 06 14:40:01 UTC 2008 >Closed-Date: Fri Apr 04 02:22:00 UTC 2008 >Last-Modified: Fri Apr 4 02:30:03 UTC 2008 >Originator: Yar Tikhiy >Release: FreeBSD 8.0-CURRENT i386 >Organization: none >Environment: System: FreeBSD behemoth.ramtel.ru 8.0-CURRENT FreeBSD 8.0-CURRENT #7: Wed Feb 6 15:46:33 MSK 2008 yar@behemoth.ramtel.ru:/mnt/obj/mnt/src/sys/BEHEMOTH2 i386 >Description: Running fsck(8) on the root file system mounted read-only (e.g., from single user mode) results in the mount upgraded to read-write. This problem seems to manifest itself only on the root file system. It doesn't matter if fsck runs in read-only (-n) or read-write mode. This issue might be related to that from PR bin/106636 as both fsck_ffs and mountd invoke nmount(2) to update the mount under the conditions described in these PRs. >How-To-Repeat: [kernel boots...] Trying to mount root from ufs:/dev/ad0s3a Enter full pathname of shell or RETURN for /bin/sh: # mount /dev/ad0s3a on / (ufs, local, read-only) devfs on /dev (devfs, local) # fsck / ** /dev/ad0s3a ** Last Mounted on / ** Root file system ** Phase 1 - Check Blocks and Sizes ** Phase 2 - Check Pathnames ** Phase 3 - Check Connectivity ** Phase 4 - Check Reference Counts ** Phase 5 - Check Cyl groups 3847 files, 190700 used, 63115 free (675 frags, 7805 blocks, 0.3% fragmentation) # mount /dev/ad0s3a on / (ufs, local) devfs on /dev (devfs, local) >Fix: >Release-Note: >Audit-Trail: From: Jaakko Heinonen To: bug-followup@FreeBSD.org, yar@comp.chem.msu.su Cc: Subject: Re: kern/120319: fsck on read-only root fs upgrades it to read-write Date: Thu, 7 Feb 2008 12:19:48 +0200 --KsGdsel6WgEHnImy Content-Type: text/plain; charset=us-ascii Content-Disposition: inline This happens because the kernel doesn't set the "ro" mount option initially for mounts in vfs_mountroot_try() (vfs_mount.c). ffs_mount() remounts a file system as read-write if the "ro" option is missing. Following patch adds the "ro" option for initial root mounts. It should fix the problem. Could you verify that? -- Jaakko --KsGdsel6WgEHnImy Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="root-remount.diff" Index: vfs_mount.c =================================================================== RCS file: /home/ncvs/src/sys/kern/vfs_mount.c,v retrieving revision 1.273 diff -u -r1.273 vfs_mount.c --- vfs_mount.c 24 Jan 2008 12:34:28 -0000 1.273 +++ vfs_mount.c 7 Feb 2008 07:33:00 -0000 @@ -1727,6 +1727,7 @@ "fstype", vfsname, "fspath", "/", "from", path, + "ro", NULL, NULL); if (error == 0) { /* @@ -2300,7 +2301,10 @@ if (cp == NULL) break; vp = va_arg(ap, const void *); - ma = mount_arg(ma, cp, vp, -1); + if (vp == NULL) + ma = mount_arg(ma, cp, NULL, 0); + else + ma = mount_arg(ma, cp, vp, -1); } va_end(ap); --KsGdsel6WgEHnImy-- From: Yar Tikhiy To: Jaakko Heinonen Cc: bug-followup@FreeBSD.org Subject: Re: kern/120319: fsck on read-only root fs upgrades it to read-write Date: Thu, 7 Feb 2008 17:58:51 +0300 On Thu, Feb 07, 2008 at 12:19:48PM +0200, Jaakko Heinonen wrote: > > This happens because the kernel doesn't set the "ro" mount option > initially for mounts in vfs_mountroot_try() (vfs_mount.c). ffs_mount() > remounts a file system as read-write if the "ro" option is missing. > > Following patch adds the "ro" option for initial root mounts. It should > fix the problem. Could you verify that? You've hit the nail on the head! Now the question is: Which of the two functions should be fixed after all? Some parts of the system seem to rely solely on MNT_RDONLY to get a read-only mount, so it might be wrong for ffs_mount() to look for the "ro" option even if MNT_RDONLY is set in the mount flags. Any ideas? Thanks! -- Yar From: Jaakko Heinonen To: Yar Tikhiy Cc: bug-followup@FreeBSD.org Subject: Re: kern/120319: fsck on read-only root fs upgrades it to read-write Date: Thu, 7 Feb 2008 23:53:23 +0200 On 2008-02-07, Yar Tikhiy wrote: > > This happens because the kernel doesn't set the "ro" mount option > > initially for mounts in vfs_mountroot_try() (vfs_mount.c). ffs_mount() > > remounts a file system as read-write if the "ro" option is missing. > > You've hit the nail on the head! Now the question is: Which of the > two functions should be fixed after all? Some parts of the system > seem to rely solely on MNT_RDONLY to get a read-only mount, so it > might be wrong for ffs_mount() to look for the "ro" option even if > MNT_RDONLY is set in the mount flags. Any ideas? Seems that msdosfs, ext2fs, nfs and zfs also rely on "ro" on remount. So changing ffs_mount() means changes for other file systems too to keep their behavior identical. For me the vfs_mountroot_try() approach seems logical because that unifies behavior of mount(8) and vfs_mountroot_try(). -- Jaakko From: Yar Tikhiy To: Jaakko Heinonen Cc: bug-followup@FreeBSD.org Subject: Re: kern/120319: fsck on read-only root fs upgrades it to read-write Date: Mon, 11 Feb 2008 17:04:43 +0300 On Thu, Feb 07, 2008 at 11:53:23PM +0200, Jaakko Heinonen wrote: > On 2008-02-07, Yar Tikhiy wrote: > > > This happens because the kernel doesn't set the "ro" mount option > > > initially for mounts in vfs_mountroot_try() (vfs_mount.c). ffs_mount() > > > remounts a file system as read-write if the "ro" option is missing. > > > > You've hit the nail on the head! Now the question is: Which of the > > two functions should be fixed after all? Some parts of the system > > seem to rely solely on MNT_RDONLY to get a read-only mount, so it > > might be wrong for ffs_mount() to look for the "ro" option even if > > MNT_RDONLY is set in the mount flags. Any ideas? > > Seems that msdosfs, ext2fs, nfs and zfs also rely on "ro" on remount. So I bet they just had the code copied from the FFS driver. > changing ffs_mount() means changes for other file systems too to keep > their behavior identical. For me the vfs_mountroot_try() approach seems > logical because that unifies behavior of mount(8) and > vfs_mountroot_try(). I'm afraid that handling MNT_RDONLY versus the "ro" option in the kernel hasn't been really consistent since the introduction of nmount(2). Why should a caller of nmount() or vfs_donmount() have to specify both MNT_RDONLY and the "ro" option? And, according to the nmount(2) manpage: MNT_RDONLY The file system should be treated as read-only; even the super-user may not write on it. Specifying MNT_UPDATE without this option will upgrade a read-only file system to read/write. In fact, it isn't 100% true as the "ro" option is dup'ed to the MNT_RDONLY flag in vfs_donmount(); i.e., it's OK to specify "ro" only. Then why is it wrong to specify MNT_RDONLY only? Now I think I'd rather fix vfs_donmount() so that it adds the "ro" option if it isn't there but MNT_RDONLY is set. The function already does so if the "rw" option is specified. But IMHO all this is dirty hackery, whose need results from non-conformity in nmount(2) framework semantics. -- Yar State-Changed-From-To: open->patched State-Changed-By: yar State-Changed-When: Thu Feb 14 17:09:29 UTC 2008 State-Changed-Why: Fixed in CURRENT by applying a variant of the patch by Jaakko Heinonen (thank you, Jaakko!) With string options replacing bit flags in nmount(), it should be the way to go. Responsible-Changed-From-To: freebsd-bugs->yar Responsible-Changed-By: yar Responsible-Changed-When: Thu Feb 14 17:09:29 UTC 2008 Responsible-Changed-Why: Taking care of my own PR as usual. :-) http://www.freebsd.org/cgi/query-pr.cgi?pr=120319 From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/120319: commit references a PR Date: Thu, 14 Feb 2008 17:04:38 +0000 (UTC) yar 2008-02-14 17:04:31 UTC FreeBSD src repository Modified files: sys/kern vfs_mount.c Log: In the new order of things dictated by nmount(2), a read-only mount is to be requested via a "ro" option. At the same time, MNT_RDONLY is gradually becoming an indicator of the current state of the FS instead of a command flag. Today passing MNT_RDONLY alone to the kernel's mount machinery will lead to various glitches. (See the PRs for examples.) Therefore mount the root FS with a "ro" option instead of the MNT_RDONLY flag. (Note that MNT_RDONLY still is added to the mount flags internally, by vfs_donmount(), if "ro" was specified.) To be able to pass "ro" cleanly to kernel_vmount(), teach the latter function to accept options with NULL values. Also correct the comment explaining how mount_arg() handles length of -1. PR: bin/106636 kern/120319 Submitted by: Jaakko Heinonen (originally) Revision Changes Path 1.274 +4 -3 src/sys/kern/vfs_mount.c _______________________________________________ cvs-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/cvs-all To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org" State-Changed-From-To: patched->open State-Changed-By: yar State-Changed-When: Mon Feb 18 20:59:18 UTC 2008 State-Changed-Why: The bug appears to be protected by sleeping dragons. Responsible-Changed-From-To: yar->freebsd-bugs Responsible-Changed-By: yar Responsible-Changed-When: Mon Feb 18 20:59:18 UTC 2008 Responsible-Changed-Why: The bug appears to be protected by sleeping dragons. http://www.freebsd.org/cgi/query-pr.cgi?pr=120319 From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/120319: commit references a PR Date: Mon, 18 Feb 2008 20:59:05 +0000 (UTC) yar 2008-02-18 20:58:57 UTC FreeBSD src repository Modified files: sbin/mount_nfs mount_nfs.c sys/kern vfs_mount.c Log: Undo the damage I did in sys/kern/vfs_mount.c #1.274 and sbin/mount_nfs/mount_nfs.c #1.76. Let the dragons sleep. Requested by: rodrigc, des PR: kern/120319 (welcome the bug back) Revision Changes Path 1.77 +0 -9 src/sbin/mount_nfs/mount_nfs.c 1.276 +1 -2 src/sys/kern/vfs_mount.c _______________________________________________ cvs-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/cvs-all To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org" State-Changed-From-To: open->suspended State-Changed-By: linimon State-Changed-When: Fri Feb 29 02:20:01 UTC 2008 State-Changed-Why: Apparently a patch was committed and then backed out. Mark this as 'suspended' so I won't trip over it again. Hat: bugmeister http://www.freebsd.org/cgi/query-pr.cgi?pr=120319 State-Changed-From-To: suspended->open State-Changed-By: rodrigc State-Changed-When: Wed Mar 5 08:00:32 UTC 2008 State-Changed-Why: The bug is real, but the proposed patch caused more problems than it solved. Responsible-Changed-From-To: freebsd-bugs->rodrigc Responsible-Changed-By: rodrigc Responsible-Changed-When: Wed Mar 5 08:00:32 UTC 2008 Responsible-Changed-Why: The bug is real, but the proposed patch caused more problems than it solved. http://www.freebsd.org/cgi/query-pr.cgi?pr=120319 From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/120319: commit references a PR Date: Wed, 5 Mar 2008 08:25:57 +0000 (UTC) rodrigc 2008-03-05 08:25:49 UTC FreeBSD src repository Modified files: sbin/fsck_ffs main.c Log: For a mounted file system which is read-only, when doing the MNT_RELOAD, pass in "ro" and "update" string mount options to nmount() instead of MNT_RDONLY and MNT_UPDATE flags. Due to the complexity of the mount parsing code especially with respect to the root file system, passing in MNT_RDONLY and MNT_UPDATE flags would do weird things and would cause fsck to convert the root file system from a read-only mount to read-write. To test: - boot into single user mode - show mounted file systems with: mount - root file system should be mounted read-only - fsck / - show mounted file systems with: mount - root file system should still be mounted read-only PR: 120319 MFC after: 1 month Reported by: yar Revision Changes Path 1.49 +3 -1 src/sbin/fsck_ffs/main.c _______________________________________________ cvs-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/cvs-all To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org" State-Changed-From-To: open->patched State-Changed-By: rodrigc State-Changed-When: Wed Mar 5 08:39:06 UTC 2008 State-Changed-Why: http://www.freebsd.org/cgi/query-pr.cgi?pr=120319 State-Changed-From-To: patched->closed State-Changed-By: rodrigc State-Changed-When: Fri Apr 4 02:21:49 UTC 2008 State-Changed-Why: http://www.freebsd.org/cgi/query-pr.cgi?pr=120319 From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/120319: commit references a PR Date: Fri, 4 Apr 2008 02:20:22 +0000 (UTC) rodrigc 2008-04-04 02:20:16 UTC FreeBSD src repository Modified files: (Branch: RELENG_7) sbin/fsck_ffs main.c Log: MFC 1.49-1.50: - do not upgrade a read-only mount to read-write when we fsck it PR: 120319 Revision Changes Path 1.47.2.1 +7 -1 src/sbin/fsck_ffs/main.c _______________________________________________ cvs-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/cvs-all To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org" >Unformatted: