[PATCH]e2fsprogs: Check device id in advance to skip fake device name

[PATCH]e2fsprogs: Check device id in advance to skip fake device name

[Guiyao]

Hi,

In some cases, using resize2fs to resize one fs will return "fail".
Reproduce steps are as follows,
1. create 2 folders, for example "mnt" and "tmp"
2. mount /dev/sdb onto tmp as tmpfs
3. mount /dev/sdb onto mnt as ext4 or other normal file system
4. try to resize /dev/sdb, it FAILED! -> "Couldn't find valid filesystem superblock."
5. if mount mnt firstly, resize2fs command will succeed.

In check_mntent_file func, firstly try to find out the input device name in mtab_file line by line,
and it will leave from loop once one item matched.
Then, check the mount point's st_dev of matched item,
if it is not same with the input device's st_dev, it will return fail.
In this case, the first matched item in mtab_file is "tmp" mount point,
it is only named as "/dev/sdb", which actually is not sdb's real mount point.
Finally, the name is matched, but st_dev is not matched, and then resize command fails.

Here, we check the st_dev immediately once the name is matched.
If st_dev not same, continue to next loop.


Signed-off-by: GuiYao <guiyao@huawei.com>
---
 lib/ext2fs/ismounted.c | 49 +++++++++++++++++-------------------------
 1 file changed, 20 insertions(+), 29 deletions(-)

diff --git a/lib/ext2fs/ismounted.c b/lib/ext2fs/ismounted.c index 6cd497dc..265d27f7 100644
--- a/lib/ext2fs/ismounted.c
+++ b/lib/ext2fs/ismounted.c
@@ -98,6 +98,9 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,  {
 	struct mntent 	*mnt;
 	struct stat	st_buf;
+#ifndef __GNU__
+	struct stat	dir_st_buf;
+#endif  /* __GNU__ */
 	errcode_t	retval = 0;
 	dev_t		file_dev=0, file_rdev=0;
 	ino_t		file_ino=0;
@@ -128,13 +131,26 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,
 	while ((mnt = getmntent (f)) != NULL) {
 		if (mnt->mnt_fsname[0] != '/')
 			continue;
-		if (strcmp(file, mnt->mnt_fsname) == 0)
+#ifndef __GNU__
+		if (stat(mnt->mnt_dir, &dir_st_buf) != 0)
+			continue;
+#endif  /* __GNU__ */
+		if (strcmp(file, mnt->mnt_fsname) == 0) { #ifndef __GNU__
+			if (file_rdev && (file_rdev == dir_st_buf.st_dev))
+				break;
+			continue;
+#else
 			break;
+#endif  /* __GNU__ */
+		}
 		if (stat(mnt->mnt_fsname, &st_buf) == 0) {
 			if (ext2fsP_is_disk_device(st_buf.st_mode)) {  #ifndef __GNU__
-				if (file_rdev && (file_rdev == st_buf.st_rdev))
-					break;
+				if (file_rdev && (file_rdev == st_buf.st_rdev)) {
+					if (file_rdev == dir_st_buf.st_dev)
+						break;
+				}
 				if (check_loop_mounted(mnt->mnt_fsname,
 						st_buf.st_rdev, file_dev,
 						file_ino) == 1)
@@ -168,32 +184,7 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,
 #endif	/* __GNU__ */
 		goto errout;
 	}
-#ifndef __GNU__ /* The GNU hurd is deficient; what else is new? */
-	/* Validate the entry in case /etc/mtab is out of date */
-	/*
-	 * We need to be paranoid, because some broken distributions
-	 * (read: Slackware) don't initialize /etc/mtab before checking
-	 * all of the non-root filesystems on the disk.
-	 */
-	if (stat(mnt->mnt_dir, &st_buf) < 0) {
-		retval = errno;
-		if (retval == ENOENT) {
-#ifdef DEBUG
-			printf("Bogus entry in %s!  (%s does not exist)\n",
-			       mtab_file, mnt->mnt_dir);
-#endif /* DEBUG */
-			retval = 0;
-		}
-		goto errout;
-	}
-	if (file_rdev && (st_buf.st_dev != file_rdev)) {
-#ifdef DEBUG
-		printf("Bogus entry in %s!  (%s not mounted on %s)\n",
-		       mtab_file, file, mnt->mnt_dir);
-#endif /* DEBUG */
-		goto errout;
-	}
-#endif /* __GNU__ */
+
 	*mount_flags = EXT2_MF_MOUNTED;
 
 #ifdef MNTOPT_RO
--
1.8.3.1

Re: [PATCH] e2fsprogs: Check device id in advance to skip fake device name

[Theodore Y. Ts'o]

Hi Guiyao,

My apologies for the delay in responding to your patch.  The situation
didn't seem to be something that would happen in real life.  (What
insane system administrator would do something like "mount -t tmpfs
/dev/sdb /tmp"?)

Also, your patch was damaged; when applied, it would result in file
that would not compile:

> +		if (strcmp(file, mnt->mnt_fsname) == 0) { #ifndef __GNU__

It was also quite a bit more complex than it needed to be, and this is
code that requires careful auditing since it has to work in a large
number of operating systems and distribution set ups.

As a result, I didn't give analyzing this patch high priority; but
I've finally got around to rewriting it.

This is the patch which I've come up.

						- Ted

From ea4d53b7b9079fd6e2cc34cf569a993a183bfbd2 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <tytso@mit.edu>
Date: Sun, 10 Nov 2019 12:11:49 -0500
Subject: [PATCH] libext2fs/ismounted.c: check device id in advance to skip
 false device names

If there is a trickster which tries to use device names as the mount
device for pseudo-file systems, the resulting /proc/mounts can confuse
ext2fs_check_mount_point().  (So far as I can tell, there's no good
reason to do this, but sysadmins do the darnest things.)

An example of this might be the following /proc/mounts excerpt:

/dev/sdb /mnt2 tmpfs rw,relatime 0 0
/dev/sdb /mnt ext4 rw,relatime 0 0

This is created via "mount -t tmpfs /dev/sdb /mnt2" followed via
"mount -t ext4 /dev/sdb /mnt".  (Normally, a sane mount of tmpfs would
use something like "mount -t tmpfs tmpfs /mnt2".)

Fix this by double checking the st_rdev of the claimed mountpoint and
match it with the dev_t of the device.  (Note that the GNU HURD
doesn't support st_rdev, so we can't solve this problem for the HURD.)

Reported-by: GuiYao <guiyao@huawei.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 lib/ext2fs/ismounted.c | 39 ++++++++++++---------------------------
 1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/lib/ext2fs/ismounted.c b/lib/ext2fs/ismounted.c
index 6cd497dc..dc37cce4 100644
--- a/lib/ext2fs/ismounted.c
+++ b/lib/ext2fs/ismounted.c
@@ -128,8 +128,19 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,
 	while ((mnt = getmntent (f)) != NULL) {
 		if (mnt->mnt_fsname[0] != '/')
 			continue;
-		if (strcmp(file, mnt->mnt_fsname) == 0)
+		if (stat(mnt->mnt_dir, &st_buf) != 0)
+			continue;
+		if (strcmp(file, mnt->mnt_fsname) == 0) {
+			if (file_rdev && (file_rdev != st_buf.st_dev)) {
+#ifdef DEBUG
+				printf("Bogus entry in %s!  "
+				       "(%s does not exist)\n",
+				       mtab_file, mnt->mnt_dir);
+#endif /* DEBUG */
+				continue;
+			}
 			break;
+		}
 		if (stat(mnt->mnt_fsname, &st_buf) == 0) {
 			if (ext2fsP_is_disk_device(st_buf.st_mode)) {
 #ifndef __GNU__
@@ -168,32 +179,6 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,
 #endif	/* __GNU__ */
 		goto errout;
 	}
-#ifndef __GNU__ /* The GNU hurd is deficient; what else is new? */
-	/* Validate the entry in case /etc/mtab is out of date */
-	/*
-	 * We need to be paranoid, because some broken distributions
-	 * (read: Slackware) don't initialize /etc/mtab before checking
-	 * all of the non-root filesystems on the disk.
-	 */
-	if (stat(mnt->mnt_dir, &st_buf) < 0) {
-		retval = errno;
-		if (retval == ENOENT) {
-#ifdef DEBUG
-			printf("Bogus entry in %s!  (%s does not exist)\n",
-			       mtab_file, mnt->mnt_dir);
-#endif /* DEBUG */
-			retval = 0;
-		}
-		goto errout;
-	}
-	if (file_rdev && (st_buf.st_dev != file_rdev)) {
-#ifdef DEBUG
-		printf("Bogus entry in %s!  (%s not mounted on %s)\n",
-		       mtab_file, file, mnt->mnt_dir);
-#endif /* DEBUG */
-		goto errout;
-	}
-#endif /* __GNU__ */
 	*mount_flags = EXT2_MF_MOUNTED;
 
 #ifdef MNTOPT_RO
-- 
2.23.0

[PATCH] e2fsprogs: Check device id in advance to skip fake device name

[Guiyao]

Hi, Theodore and All,

It's a friendly reminder, maybe you are too busy to missed this email. :-)

In some cases, using resize2fs to resize one fs will return "fail".
Reproduce steps are as follows,
1. create 2 folders, for example "mnt" and "tmp"
2. mount /dev/sdb onto tmp as tmpfs
3. mount /dev/sdb onto mnt as ext4 or other normal file system 4. try to resize /dev/sdb, it FAILED! -> "Couldn't find valid filesystem superblock."
5. if mount mnt firstly, resize2fs command will succeed.

In check_mntent_file func, firstly try to find out the input device name in mtab_file line by line, and it will leave from loop once one item matched.
Then, check the mount point's st_dev of matched item, if it is not same with the input device's st_dev, it will return fail.
In this case, the first matched item in mtab_file is "tmp" mount point, it is only named as "/dev/sdb", which actually is not sdb's real mount point.
Finally, the name is matched, but st_dev is not matched, and then resize command fails.

Here, we check the st_dev immediately once the name is matched.
If st_dev not same, continue to next loop.


Signed-off-by: GuiYao <guiyao@huawei.com>
---
 lib/ext2fs/ismounted.c | 49 +++++++++++++++++-------------------------
 1 file changed, 20 insertions(+), 29 deletions(-)

diff --git a/lib/ext2fs/ismounted.c b/lib/ext2fs/ismounted.c index 6cd497dc..265d27f7 100644
--- a/lib/ext2fs/ismounted.c
+++ b/lib/ext2fs/ismounted.c
@@ -98,6 +98,9 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,  {
 	struct mntent 	*mnt;
 	struct stat	st_buf;
+#ifndef __GNU__
+	struct stat	dir_st_buf;
+#endif  /* __GNU__ */
 	errcode_t	retval = 0;
 	dev_t		file_dev=0, file_rdev=0;
 	ino_t		file_ino=0;
@@ -128,13 +131,26 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,
 	while ((mnt = getmntent (f)) != NULL) {
 		if (mnt->mnt_fsname[0] != '/')
 			continue;
-		if (strcmp(file, mnt->mnt_fsname) == 0)
+#ifndef __GNU__
+		if (stat(mnt->mnt_dir, &dir_st_buf) != 0)
+			continue;
+#endif  /* __GNU__ */
+		if (strcmp(file, mnt->mnt_fsname) == 0) { #ifndef __GNU__
+			if (file_rdev && (file_rdev == dir_st_buf.st_dev))
+				break;
+			continue;
+#else
 			break;
+#endif  /* __GNU__ */
+		}
 		if (stat(mnt->mnt_fsname, &st_buf) == 0) {
 			if (ext2fsP_is_disk_device(st_buf.st_mode)) {  #ifndef __GNU__
-				if (file_rdev && (file_rdev == st_buf.st_rdev))
-					break;
+				if (file_rdev && (file_rdev == st_buf.st_rdev)) {
+					if (file_rdev == dir_st_buf.st_dev)
+						break;
+				}
 				if (check_loop_mounted(mnt->mnt_fsname,
 						st_buf.st_rdev, file_dev,
 						file_ino) == 1)
@@ -168,32 +184,7 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,
 #endif	/* __GNU__ */
 		goto errout;
 	}
-#ifndef __GNU__ /* The GNU hurd is deficient; what else is new? */
-	/* Validate the entry in case /etc/mtab is out of date */
-	/*
-	 * We need to be paranoid, because some broken distributions
-	 * (read: Slackware) don't initialize /etc/mtab before checking
-	 * all of the non-root filesystems on the disk.
-	 */
-	if (stat(mnt->mnt_dir, &st_buf) < 0) {
-		retval = errno;
-		if (retval == ENOENT) {
-#ifdef DEBUG
-			printf("Bogus entry in %s!  (%s does not exist)\n",
-			       mtab_file, mnt->mnt_dir);
-#endif /* DEBUG */
-			retval = 0;
-		}
-		goto errout;
-	}
-	if (file_rdev && (st_buf.st_dev != file_rdev)) {
-#ifdef DEBUG
-		printf("Bogus entry in %s!  (%s not mounted on %s)\n",
-		       mtab_file, file, mnt->mnt_dir);
-#endif /* DEBUG */
-		goto errout;
-	}
-#endif /* __GNU__ */
+
 	*mount_flags = EXT2_MF_MOUNTED;
 
 #ifdef MNTOPT_RO
--
1.8.3.1

[PATCH] e2fsprogs: Check device id in advance to skip fake device name

[Guiyao]

Hi,

In some cases, using resize2fs to resize one fs will return "fail".
Reproduce steps are as follows,
1. create 2 folders, for example "mnt" and "tmp"
2. mount /dev/sdb onto tmp as tmpfs
3. mount /dev/sdb onto mnt as ext4 or other normal file system 4. try to resize /dev/sdb, it FAILED! -> "Couldn't find valid filesystem superblock."
5. if mount mnt firstly, resize2fs command will succeed.

In check_mntent_file func, firstly try to find out the input device name in mtab_file line by line, and it will leave from loop once one item matched.
Then, check the mount point's st_dev of matched item, if it is not same with the input device's st_dev, it will return fail.
In this case, the first matched item in mtab_file is "tmp" mount point, it is only named as "/dev/sdb", which actually is not sdb's real mount point.
Finally, the name is matched, but st_dev is not matched, and then resize command fails.

Here, we check the st_dev immediately once the name is matched.
If st_dev not same, continue to next loop.


Signed-off-by: GuiYao <guiyao@huawei.com>
---
 lib/ext2fs/ismounted.c | 49 +++++++++++++++++-------------------------
 1 file changed, 20 insertions(+), 29 deletions(-)

diff --git a/lib/ext2fs/ismounted.c b/lib/ext2fs/ismounted.c index 6cd497dc..265d27f7 100644
--- a/lib/ext2fs/ismounted.c
+++ b/lib/ext2fs/ismounted.c
@@ -98,6 +98,9 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,  {
 	struct mntent 	*mnt;
 	struct stat	st_buf;
+#ifndef __GNU__
+	struct stat	dir_st_buf;
+#endif  /* __GNU__ */
 	errcode_t	retval = 0;
 	dev_t		file_dev=0, file_rdev=0;
 	ino_t		file_ino=0;
@@ -128,13 +131,26 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,
 	while ((mnt = getmntent (f)) != NULL) {
 		if (mnt->mnt_fsname[0] != '/')
 			continue;
-		if (strcmp(file, mnt->mnt_fsname) == 0)
+#ifndef __GNU__
+		if (stat(mnt->mnt_dir, &dir_st_buf) != 0)
+			continue;
+#endif  /* __GNU__ */
+		if (strcmp(file, mnt->mnt_fsname) == 0) { #ifndef __GNU__
+			if (file_rdev && (file_rdev == dir_st_buf.st_dev))
+				break;
+			continue;
+#else
 			break;
+#endif  /* __GNU__ */
+		}
 		if (stat(mnt->mnt_fsname, &st_buf) == 0) {
 			if (ext2fsP_is_disk_device(st_buf.st_mode)) {  #ifndef __GNU__
-				if (file_rdev && (file_rdev == st_buf.st_rdev))
-					break;
+				if (file_rdev && (file_rdev == st_buf.st_rdev)) {
+					if (file_rdev == dir_st_buf.st_dev)
+						break;
+				}
 				if (check_loop_mounted(mnt->mnt_fsname,
 						st_buf.st_rdev, file_dev,
 						file_ino) == 1)
@@ -168,32 +184,7 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,
 #endif	/* __GNU__ */
 		goto errout;
 	}
-#ifndef __GNU__ /* The GNU hurd is deficient; what else is new? */
-	/* Validate the entry in case /etc/mtab is out of date */
-	/*
-	 * We need to be paranoid, because some broken distributions
-	 * (read: Slackware) don't initialize /etc/mtab before checking
-	 * all of the non-root filesystems on the disk.
-	 */
-	if (stat(mnt->mnt_dir, &st_buf) < 0) {
-		retval = errno;
-		if (retval == ENOENT) {
-#ifdef DEBUG
-			printf("Bogus entry in %s!  (%s does not exist)\n",
-			       mtab_file, mnt->mnt_dir);
-#endif /* DEBUG */
-			retval = 0;
-		}
-		goto errout;
-	}
-	if (file_rdev && (st_buf.st_dev != file_rdev)) {
-#ifdef DEBUG
-		printf("Bogus entry in %s!  (%s not mounted on %s)\n",
-		       mtab_file, file, mnt->mnt_dir);
-#endif /* DEBUG */
-		goto errout;
-	}
-#endif /* __GNU__ */
+
 	*mount_flags = EXT2_MF_MOUNTED;
 
 #ifdef MNTOPT_RO
--
1.8.3.1