Re: [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 30 Jun 2011 16:10:06 -0500
Will Drewry <wad@chromium.org> wrote:

> Expand root=PARTUUID=UUID syntax to support selecting a root partition
> by integer offset from a known, unique partition.  This approach
> provides similar properties to specifying a device and partition number,
> but using the UUID as the unique path prior to evaluating the offset.
> 
> For example,
>   root=PARTUUID=99DE9194-FC15-4223-9192-FC243948F88B/PARTNROFF=1
> selects the partition with UUID 99DE.. then select the next
> partition.
> 
> This change is motivated by a particular usecase in Chromium OS where
> the bootloader can easily determine what partition it is on (by UUID)
> but doesn't perform general partition table walking.
> 
> That said, support for this model provides a direct mechanism for the
> user to modify the root partition to boot without specifically needing
> to extract each UUID or update the bootloader explicitly when the root
> partition UUID is changed (if it is recreated to be larger, for
> instance).  Pinning to a /boot-style partition UUID allows the arbitrary
> root partition reconfiguration/modifications with slightly less
> ambiguity than just [dev][partition] and less stringency than the
> specific root partition UUID.
> 
> ...
>
>  static dev_t devt_from_partuuid(char *uuid_str)
> @@ -98,6 +101,22 @@ static dev_t devt_from_partuuid(char *uuid_str)
>  	dev_t res = 0;
>  	struct device *dev = NULL;
>  	u8 uuid[16];
> +	struct gendisk *disk;
> +	struct hd_struct *part;
> +	int offset = 0;
> +
> +	if (strlen(uuid_str) < 36)
> +		goto done;

I think this secretly changes behaviour?  Previously the code would have
accepted a less-than-36-byte UUID and would have done <something> with
it.  Now, it fails.

What was <something>, and what is the reason for this (undocumented!)
change?

> +	/* Check for optional partition number offset attributes. */
> +	if (uuid_str[36]) {
> +		/* Explicitly fail on poor PARTUUID syntax. */
> +		if (sscanf(&uuid_str[36], "/PARTNROFF=%d", &offset) != 1) {
> +			printk(KERN_ERR "VFS: PARTUUID= is invalid.\n"
> +			 "Expected PARTUUID=<valid-uuid-id>[/PARTNROFF=%%d]\n");

The check isn't complete - afacit input of the form PARTNROFF=42foo
will be treated as PARTNROFF=42?

>
> ...
>