[PATCH 1/2] init: Stricter checking of major:minor root= values

[PATCH 1/2] init: Stricter checking of major:minor root= values

[Dan Ehrenberg]

In the kernel command-line, previously, root=1:2jakshflaksjdhfa would
be accepted and interpreted just like root=1:2. This patch adds
stricter checking so that additional characters after major:minor are
rejected by root=.

The patch also makes name_to_dev_t take a const char * rather than a
char * for improved possibilities for checking const correctness.

The goal of this patch is to help in unifying dm's interpretation of
its block device argument with the interpretation in other parts of
the kernel. dm rejects malformed major:minor pairs, and it seems
reasonable that root= should reject them as well.

Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
---
 include/linux/mount.h | 2 +-
 init/do_mounts.c      | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/linux/mount.h b/include/linux/mount.h
index c2c561d..bca086d 100644
--- a/include/linux/mount.h
+++ b/include/linux/mount.h
@@ -92,6 +92,6 @@ extern struct vfsmount *vfs_kern_mount(struct file_system_type *type,
 extern void mnt_set_expiry(struct vfsmount *mnt, struct list_head *expiry_list);
 extern void mark_mounts_for_expiry(struct list_head *mounts);
 
-extern dev_t name_to_dev_t(char *name);
+extern dev_t name_to_dev_t(const char *name);
 
 #endif /* _LINUX_MOUNT_H */
diff --git a/init/do_mounts.c b/init/do_mounts.c
index eb41008..7e06620 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -207,7 +207,7 @@ done:
  *	bangs.
  */
 
-dev_t name_to_dev_t(char *name)
+dev_t name_to_dev_t(const char *name)
 {
 	char s[32];
 	char *p;
@@ -226,8 +226,9 @@ dev_t name_to_dev_t(char *name)
 
 	if (strncmp(name, "/dev/", 5) != 0) {
 		unsigned maj, min;
+		char dummy;
 
-		if (sscanf(name, "%u:%u", &maj, &min) == 2) {
+		if (sscanf(name, "%u:%u%c", &maj, &min, &dummy) == 2) {
 			res = MKDEV(maj, min);
 			if (maj != MAJOR(res) || min != MINOR(res))
 				goto fail;
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 2/2] dm: Get devices using name_to_dev_t

[Dan Ehrenberg]

If a device is used as the root filesystem, it can't be built
off of devices which are within the root filesystem (just like
command line arguments to root=). For this reason, Linux has a
pseudo-filesystem for root= and md initialization based on the
function name_to_dev_t, which handles different ways of specifying
devices including PARTUUID and major:minor.

This patch applies name_to_dev_t to dm initialization. Rather
than assuming that all things which are not major:minor are paths
in an already-mounted filesystem, this patch first attempts
to look up the device in the filesystem, and applies name_to_dev_t
if it is not found there.

In terms of backwards compatibility, there are some cases where
behavior will be different:
- If you have a file in the current working directory named 1:2 and
  you initialze DM there, then it will try to use that file rather
  than the disk with that major/minor pair as a backing device.
- Similarly for other bdev types which name_to_dev_t knows how to
  interpret, the previous behavior was to repeatedly check for the
  existence of the file (e.g., while waiting for rootfs to come up)
  but the new behavior is to use the name_to_dev_t interpretation.
  For example, if you have a file named /dev/ubiblock0_0 which is
  a symlink to /dev/sda3, but it is not yet present when dm starts
  to initialize, then the name_to_dev_t interpretation will take
  precedence.
I believe these incompatibilities would only show up in really
strange setups with bad practices and we don't have to worry about
them.

Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
---
 drivers/md/dm-table.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 3afae9e..3ce1e01 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -18,6 +18,7 @@
 #include <linux/mutex.h>
 #include <linux/delay.h>
 #include <linux/atomic.h>
+#include <linux/mount.h>
 
 #define DM_MSG_PREFIX "table"
 
@@ -372,23 +373,18 @@ int dm_get_device(struct dm_target *ti, const char *path, fmode_t mode,
 	int r;
 	dev_t uninitialized_var(dev);
 	struct dm_dev_internal *dd;
-	unsigned int major, minor;
 	struct dm_table *t = ti->table;
-	char dummy;
+	struct block_device *bdev;
 
 	BUG_ON(!t);
 
-	if (sscanf(path, "%u:%u%c", &major, &minor, &dummy) == 2) {
-		/* Extract the major/minor numbers */
-		dev = MKDEV(major, minor);
-		if (MAJOR(dev) != major || MINOR(dev) != minor)
-			return -EOVERFLOW;
+	/* convert the path to a device */
+	bdev = lookup_bdev(path);
+	if (IS_ERR(bdev)) {
+		dev = name_to_dev_t(path);
+		if (!dev)
+			return -ENODEV;
 	} else {
-		/* convert the path to a device */
-		struct block_device *bdev = lookup_bdev(path);
-
-		if (IS_ERR(bdev))
-			return PTR_ERR(bdev);
 		dev = bdev->bd_dev;
 		bdput(bdev);
 	}
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH 2/2] dm: Get devices using name_to_dev_t

[Mike Snitzer]

On Wed, Jan 28 2015 at  5:45pm -0500,
Dan Ehrenberg <dehrenberg@chromium.org> wrote:

> If a device is used as the root filesystem, it can't be built
> off of devices which are within the root filesystem (just like
> command line arguments to root=). For this reason, Linux has a
> pseudo-filesystem for root= and md initialization based on the
> function name_to_dev_t, which handles different ways of specifying
> devices including PARTUUID and major:minor.
> 
> This patch applies name_to_dev_t to dm initialization. Rather
> than assuming that all things which are not major:minor are paths
> in an already-mounted filesystem, this patch first attempts
> to look up the device in the filesystem, and applies name_to_dev_t
> if it is not found there.
> 
> In terms of backwards compatibility, there are some cases where
> behavior will be different:
> - If you have a file in the current working directory named 1:2 and
>   you initialze DM there, then it will try to use that file rather
>   than the disk with that major/minor pair as a backing device.
> - Similarly for other bdev types which name_to_dev_t knows how to
>   interpret, the previous behavior was to repeatedly check for the
>   existence of the file (e.g., while waiting for rootfs to come up)
>   but the new behavior is to use the name_to_dev_t interpretation.
>   For example, if you have a file named /dev/ubiblock0_0 which is
>   a symlink to /dev/sda3, but it is not yet present when dm starts
>   to initialize, then the name_to_dev_t interpretation will take
>   precedence.
> I believe these incompatibilities would only show up in really
> strange setups with bad practices and we don't have to worry about
> them.
> 
> Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
> ---
>  drivers/md/dm-table.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 3afae9e..3ce1e01 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -18,6 +18,7 @@
>  #include <linux/mutex.h>
>  #include <linux/delay.h>
>  #include <linux/atomic.h>
> +#include <linux/mount.h>
>  
>  #define DM_MSG_PREFIX "table"
>  
> @@ -372,23 +373,18 @@ int dm_get_device(struct dm_target *ti, const char *path, fmode_t mode,
>  	int r;
>  	dev_t uninitialized_var(dev);
>  	struct dm_dev_internal *dd;
> -	unsigned int major, minor;
>  	struct dm_table *t = ti->table;
> -	char dummy;
> +	struct block_device *bdev;
>  
>  	BUG_ON(!t);
>  
> -	if (sscanf(path, "%u:%u%c", &major, &minor, &dummy) == 2) {
> -		/* Extract the major/minor numbers */
> -		dev = MKDEV(major, minor);
> -		if (MAJOR(dev) != major || MINOR(dev) != minor)
> -			return -EOVERFLOW;
> +	/* convert the path to a device */
> +	bdev = lookup_bdev(path);
> +	if (IS_ERR(bdev)) {
> +		dev = name_to_dev_t(path);
> +		if (!dev)
> +			return -ENODEV;
>  	} else {
> -		/* convert the path to a device */
> -		struct block_device *bdev = lookup_bdev(path);
> -
> -		if (IS_ERR(bdev))
> -			return PTR_ERR(bdev);
>  		dev = bdev->bd_dev;
>  		bdput(bdev);
>  	}

name_to_dev_t isn't an exported symbol so it is undefined if DM is built
as a module (as many distros do).

I was going to pick this up to submit via linux-dm.git but lost
conviction once I hit compile errors and it became clear others need to
weigh in on the broader use of name_to_dev_t().

Please Cc Al Viro on v2 (and any others that are returned by
scripts/get_maintainer.pl).

Mike