Re: [PATCH] Fix potential truncation of mdraid device list

[Petr Vorel <pvorel@suse.cz>]

Hi,

> The consumer of grub_util_raid_getmembers() expects a NULL terminated list
> of device names. It has no idea about how many devices are registered in
> the array; it only cares about active devices. As a result, there cannot
> be gaps in the list, otherwise the first listed inactive device will cause
> all remaining devices to effectively vanish. This is especially
> troublesome if a root filesystem were on an array with the first device
> being a hot spare: the array would appear to have no disks and the root
> filesystem would become invisible to grub.

> Fixes: 49de079bbe1c ("... (grub_util_raid_getmembers): Handle "removed" disks")
> Fixes: https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1912043
> Fixes: https://savannah.gnu.org/bugs/index.php?59887
> Signed-off-by: Kees Cook <kees@ubuntu.com>

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> Index: grub2-2.04/grub-core/osdep/linux/getroot.c
> ===================================================================
> --- grub2-2.04.orig/grub-core/osdep/linux/getroot.c
> +++ grub2-2.04/grub-core/osdep/linux/getroot.c
> @@ -170,21 +170,21 @@ grub_util_raid_getmembers (const char *n

>    devicelist = xcalloc (info.nr_disks + 1, sizeof (char *));

> -  for (i = 0, j = 0; j < info.nr_disks; i++)
> +  for (i = 0, j = 0; i < info.nr_disks; i++)
>      {
>        disk.number = i;
>        ret = ioctl (fd, GET_DISK_INFO, &disk);
>        if (ret != 0)
>  	grub_util_error (_("ioctl GET_DISK_INFO error: %s"), strerror (errno));
> -      
> +
>        if (disk.state & (1 << MD_DISK_REMOVED))
>  	continue;

> -      if (disk.state & (1 << MD_DISK_ACTIVE))
> -	devicelist[j] = grub_find_device (NULL,
> -					  makedev (disk.major, disk.minor));
> -      else
> -	devicelist[j] = NULL;
> +      if (!(disk.state & (1 << MD_DISK_ACTIVE)))
> +	continue;
> +
> +      devicelist[j] = grub_find_device (NULL,
> +					makedev (disk.major, disk.minor));
>        j++;
>      }