Re: [PATCH 14/24] init: clear root_wait on all invalid root= strings

From: Guenter Roeck <linux@roeck-us.net>
To: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Mike Snitzer <snitzer@kernel.org>,
	Joern Engel <joern@lazybastard.org>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Pavel Machek <pavel@ucw.cz>,
	dm-devel@redhat.com, linux-kernel@vger.kernel.org,
	linux-block@vger.kernel.org, linux-mtd@lists.infradead.org,
	linux-pm@vger.kernel.org
Subject: Re: [PATCH 14/24] init: clear root_wait on all invalid root= strings
Date: Thu, 22 Jun 2023 06:54:41 -0700	[thread overview]
Message-ID: <8e6c8365-5c2b-2bad-bf3c-df2d65cc8afa@roeck-us.net> (raw)
In-Reply-To: <20230622060001.GA8351@lst.de>

On 6/21/23 23:00, Christoph Hellwig wrote:
> Hi Guenter,
> 
> can you try this patch?
> 
> diff --git a/block/early-lookup.c b/block/early-lookup.c
> index a5be3c68ed079c..66e4514d671179 100644
> --- a/block/early-lookup.c
> +++ b/block/early-lookup.c
> @@ -174,7 +174,7 @@ static int __init devt_from_devname(const char *name, dev_t *devt)
>   	while (p > s && isdigit(p[-1]))
>   		p--;
>   	if (p == s || !*p || *p == '0')
> -		return -EINVAL;
> +		return -ENODEV;
>   
>   	/* try disk name without <part number> */
>   	part = simple_strtoul(p, NULL, 10);

Not completely. Tests with root=/dev/sda still fail.

"name" passed to devt_from_devname() is "sda".

        for (p = s; *p; p++) {
                 if (*p == '/')
                         *p = '!';
         }

advances 'p' to the end of the string.

         while (p > s && isdigit(p[-1]))
		p--;

moves it back to point to the first digit (if there is one).

         if (p == s || !*p || *p == '0')
		return -EINVAL;

then fails because *p is 0. In other words, the function only accepts
drive names with digits at the end (and the first digit must not be '0').

I don't recall how I hit the other condition earlier. I have various
"/dev/mmcblkX" in my tests, where X can be any number including 0.
Maybe those fail randomly as well.

Overall I am not sure though what an "invalid" devicename is supposed
to be in this context. I have "sda", "sr0", "vda", "mtdblkX",
"nvme0n1", "mmcblkX", and "hda". Why would any of those not be eligible
for "rootwait" ?

In practice, everything not ending with a digit, or ending with
'0', fails the first test. Everything ending with a digit > 0
fails the second test. But "humptydump3p4" passes all those tests.

Guenter

---
#include <stdio.h>
#include <string.h>
#include <ctype.h>
#include <stdlib.h>

#define EINVAL1	1
#define EINVAL2	2
#define EINVAL3	3
#define ENODEV	4

static int devt_from_devname(const char *name)
{
         int part;
         char s[32];
         char *p;

         if (strlen(name) > 31)
                 return EINVAL1;

         strcpy(s, name);
         for (p = s; *p; p++) {
                 if (*p == '/')
                         *p = '!';
         }

         /*
          * Try non-existent, but valid partition, which may only exist after
          * opening the device, like partitioned md devices.
          */
         while (p > s && isdigit(p[-1]))
                 p--;
         if (p == s || !*p || *p == '0') {
                 return EINVAL2;
         }

         /* try disk name without <part number> */
         part = strtoul(p, NULL, 10);
         *p = '\0';

         /* try disk name without p<part number> */
         if (p < s + 2 || !isdigit(p[-2]) || p[-1] != 'p') {
                 return EINVAL3;
         }
         return ENODEV;
}

char *devnames[] = {
     "sda",
     "sda1",
     "mmcblk0",
     "mmcblk1",
     "mtdblk0",
     "mtdblk1",
     "vda",
     "hda",
     "nvme0n1",
     "sr0",
     "sr1",
     "humptydump3p4",
     NULL
};

int main(int argc, char **argv)
{
	char *str;
	int i;

	for (i = 0, str = devnames[0]; str; str = devnames[++i]) {
	    printf("%s: %d\n", str, devt_from_devname(str));
	}
}