linux-ext4 - [PATCH] libext2fs/ismounted.c: check open(O

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20200225143445.13182-1-lczerner@redhat.com>
Date:   Tue, 25 Feb 2020 15:34:45 +0100
From:   Lukas Czerner <lczerner@...hat.com>
To:     linux-ext4@...r.kernel.org
Cc:     tytso@....edu, Zdenek Kabelac <zkabelac@...hat.com>,
        Karel Zak <kzak@...hat.com>
Subject: [PATCH] libext2fs/ismounted.c: check open(O_EXCL) before mntent file

Currently the ext2fs_check_mount_point() will use the open(O_EXCL) check
on linux after all the other checks are done. However it is not
necessary to check mntent if open(O_EXCL) succeeds because it means that
the device is not mounted.

Moreover the commit ea4d53b7 introduced a regression where a following
set of commands fails:

vgcreate mygroup /dev/sda
lvcreate -L 1G -n lvol0 mygroup
mkfs.ext4 /dev/mygroup/lvol0
mount /dev/mygroup/lvol0 /mnt
lvrename /dev/mygroup/lvol0 /dev/mygroup/lvol1
lvcreate -L 1G -n lvol0 mygroup
mkfs.ext4 /dev/mygroup/lvol0   <<<--- This fails

It fails because it thinks that /dev/mygroup/lvol0 is mounted because
the device name in /proc/mounts is not updated following the lvrename.

Move the open(O_EXCL) check before the mntent check and return
immediatelly if the device is not busy.

Fixes: ea4d53b7 ("libext2fs/ismounted.c: check device id in advance to skip false device names")
Signed-off-by: Lukas Czerner <lczerner@...hat.com>
Reported-by: Zdenek Kabelac <zkabelac@...hat.com>
Reported-by: Karel Zak <kzak@...hat.com>
---
 lib/ext2fs/ismounted.c | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/lib/ext2fs/ismounted.c b/lib/ext2fs/ismounted.c
index c0215692..f4ef012f 100644
--- a/lib/ext2fs/ismounted.c
+++ b/lib/ext2fs/ismounted.c
@@ -352,6 +352,7 @@ errcode_t ext2fs_check_mount_point(const char *device, int *mount_flags,
 				  char *mtpt, int mtlen)
 {
 	errcode_t	retval = 0;
+	int 		busy = 0;
 
 	if (getenv("EXT2FS_PRETEND_RO_MOUNT")) {
 		*mount_flags = EXT2_MF_MOUNTED | EXT2_MF_READONLY;
@@ -366,6 +367,29 @@ errcode_t ext2fs_check_mount_point(const char *device, int *mount_flags,
 		return 0;
 	}
 
+#ifdef __linux__ /* This only works on Linux 2.6+ systems */
+	{
+		struct stat st_buf;
+
+		if (stat(device, &st_buf) == 0 &&
+		    ext2fsP_is_disk_device(st_buf.st_mode)) {
+			int fd = open(device, O_RDONLY | O_EXCL);
+
+			if (fd >= 0) {
+				/*
+				 * The device is not busy so it's
+				 * definitelly not mounted. No need to
+				 * to perform any more checks.
+				 */
+				close(fd);
+				*mount_flags = 0;
+				return 0;
+			} else if (errno == EBUSY)
+				busy = 1;
+		}
+	}
+#endif
+
 	if (is_swap_device(device)) {
 		*mount_flags = EXT2_MF_MOUNTED | EXT2_MF_SWAP;
 		strncpy(mtpt, "<swap>", mtlen);
@@ -386,21 +410,8 @@ errcode_t ext2fs_check_mount_point(const char *device, int *mount_flags,
 	if (retval)
 		return retval;
 
-#ifdef __linux__ /* This only works on Linux 2.6+ systems */
-	{
-		struct stat st_buf;
-
-		if (stat(device, &st_buf) == 0 &&
-		    ext2fsP_is_disk_device(st_buf.st_mode)) {
-			int fd = open(device, O_RDONLY | O_EXCL);
-
-			if (fd >= 0)
-				close(fd);
-			else if (errno == EBUSY)
-				*mount_flags |= EXT2_MF_BUSY;
-		}
-	}
-#endif
+	if (busy)
+		*mount_flags |= EXT2_MF_BUSY;
 
 	return 0;
 }
-- 
2.21.1