[PATCH] always set EXCLUSIVE flag for modifying e2fsck

[PATCH] always set EXCLUSIVE flag for modifying e2fsck

[Andreas Dilger]

The checks done by e2fsck for mounted vs. read-only runs is confusing.

On the one hand, if e2fsck is NOT run with the "-n" flag (i.e. it might
modify the filesystem, there is no guarantee that it will open the
filesystem with the EXCLUSIVE flag (i.e. O_EXCL) to prevent the block
device from being used (in most cases = mounted).

On the other hand, if the filesystem IS mounted it also does NOT set
the EXCLUSIVE flag to prevent it from clobbering an in-use filesystem.
That seems like a bad choice also.

On the gripping hand, if e2fsck IS run with "-n" (i.e. read-only),
and the /etc/fstab or /proc/mounts does not report the same block
device to match the mountpoint (which happens for Lustre, and can
also happen if there is an overlay mount) then the e2fsck thinks
the filesystem is unmounted, but fails because the EXCLUSIVE flag
is set even though it is running read-only.

Change the logic here so that EXCLUSIVE is ALWAYS set when e2fsck
might modify the filesystem, regardless of whether the filesystem
is mounted or not.

Index: e2fsprogs-1.41.6/e2fsck/unix.c
===================================================================
--- e2fsprogs-1.41.6.orig/e2fsck/unix.c
+++ e2fsprogs-1.41.6/e2fsck/unix.c
@@ -1230,9 +1230,7 @@ restart:
 		io_ptr = unix_io_manager;
 	flags = EXT2_FLAG_NOFREE_ON_ERROR;
        if ((ctx->options & E2F_OPT_READONLY) == 0)
-		flags |= EXT2_FLAG_RW;
-	if ((ctx->mount_flags & EXT2_MF_MOUNTED) == 0)
-		flags |= EXT2_FLAG_EXCLUSIVE;
+		flags |= EXT2_FLAG_RW | EXT2_FLAG_EXCLUSIVE;
 
 	retval = try_open_fs(ctx, flags, io_ptr, &fs);
 

If we want to assume that check_mount() will abort if the filesystem
is mounted, unless the user wants to shoot themselves in the foot, then 
the above patch could instead be modified to clear EXT2_FLAG_EXCLUSIVE
if the MF_MOUNTED flag IS set, per below.

Index: e2fsprogs-1.41.6/e2fsck/unix.c
===================================================================
--- e2fsprogs-1.41.6.orig/e2fsck/unix.c
+++ e2fsprogs-1.41.6/e2fsck/unix.c
@@ -1230,9 +1230,10 @@ restart:
 		io_ptr = unix_io_manager;
 	flags = EXT2_FLAG_NOFREE_ON_ERROR;
        if ((ctx->options & E2F_OPT_READONLY) == 0)
-		flags |= EXT2_FLAG_RW;
-	if ((ctx->mount_flags & EXT2_MF_MOUNTED) != 0)
-		flags |= EXT2_FLAG_EXCLUSIVE;
+		flags |= EXT2_FLAG_RW | EXT2_FLAG_EXCLUSIVE;
+	/* we will have aborted in check_mount() unless user asks for this */
+	if ((ctx->mount_flags & EXT2_MF_MOUNTED) != 0)
+		flags &= ~EXT2_FLAG_EXCLUSIVE;
 
 	retval = try_open_fs(ctx, flags, io_ptr, &fs);
 

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: [PATCH] always set EXCLUSIVE flag for modifying e2fsck

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1165 bytes --]

The checks done by e2fsck for mounted vs. read-only runs is confusing.

On the one hand, if e2fsck is NOT run with the "-n" flag (i.e. it might
modify the filesystem), there is no guarantee that it will open the
filesystem with the EXCLUSIVE flag (i.e. O_EXCL) to prevent the block
device from being used later (i.e. mounted while e2fsck is running).

On the other hand, if e2fsck IS run with "-n" (i.e. read-only),
and the /etc/fstab or /proc/mounts does not report the same block
device to match the mountpoint (which happens for Lustre, and can
also happen if there is an overlay mount) then the e2fsck incorrectly
thinks the filesystem is unmounted, but fails because the EXCLUSIVE
flag is set even though it is running read-only.

Fix this so that "e2fsck -n" never sets EXCLUSIVE, and when run without
"-n" it will always set EXCLUSIVE unless the user answered "yes" to:

         /dev/sda1 is mounted.

         WARNING!!!  Running e2fsck on a mounted filesystem may cause
         SEVERE filesystem damage.

         Do you really want to continue (y/n)?


Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

[-- Attachment #2: e2fsprogs-e2fsck_exclusive.patch --]
[-- Type: application/octet-stream, Size: 2089 bytes --]

The checks done by e2fsck for mounted vs. read-only runs is confusing.

On the one hand, if e2fsck is NOT run with the "-n" flag (i.e. it might
modify the filesystem), there is no guarantee that it will open the
filesystem with the EXCLUSIVE flag (i.e. O_EXCL) to prevent the block
device from being used later (i.e. mounted while e2fsck is running).

On the other hand, if e2fsck is run with "-n" (i.e. read-only),
and the /etc/fstab or /proc/mounts does not report the same block
device to match the mountpoint (which happens for Lustre, and can
also happen if there is an overlay mount) then the e2fsck thinks
the filesystem is unmounted, but fails because the EXCLUSIVE flag
is set even though it is running read-only.

Fix this so that "e2fsck -n" never sets EXCLUSIVE, and when run without
"-n" it will always set EXCLUSIVE unless the user answered "yes" to:

        /dev/sda1 is mounted.  

        WARNING!!!  Running e2fsck on a mounted filesystem may cause
        SEVERE filesystem damage.

        Do you really want to continue (y/n)?


Signed-off-by: Andreas Dilger <adilger@sun.com>

Index: e2fsprogs-1.41.6/e2fsck/unix.c
===================================================================
--- e2fsprogs-1.41.6.orig/e2fsck/unix.c
+++ e2fsprogs-1.41.6/e2fsck/unix.c
@@ -1230,9 +1230,14 @@ restart:
 		io_ptr = unix_io_manager;
 	flags = EXT2_FLAG_NOFREE_ON_ERROR;
 	if ((ctx->options & E2F_OPT_READONLY) == 0)
-		flags |= EXT2_FLAG_RW;
-	if ((ctx->mount_flags & EXT2_MF_MOUNTED) == 0)
-		flags |= EXT2_FLAG_EXCLUSIVE;
+		flags |= EXT2_FLAG_RW | EXT2_FLAG_EXCLUSIVE;
+	/* We would have aborted above in check_mount() unless the user
+	 * asks for this.  If the device is mounted, but it wasn't found in
+	 * /etc/mtab or /proc/mounts by ext2fs_check_if_mounted(), or it is
+	 * not the root filesystem then EXT2_MF_MOUNTED would not be set,
+	 * and we do not clear EXT2_FLAG_EXCLUSIVE. */
+	if ((ctx->mount_flags & EXT2_MF_MOUNTED) != 0)
+		flags &= ~EXT2_FLAG_EXCLUSIVE;
 
 	retval = try_open_fs(ctx, flags, io_ptr, &fs);