* [PATCH] Fix potential truncation of mdraid device list
@ 2021-01-16 11:04 Kees Cook
2021-01-19 21:51 ` Petr Vorel
0 siblings, 1 reply; 2+ messages in thread
From: Kees Cook @ 2021-01-16 11:04 UTC (permalink / raw)
To: grub-devel
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>
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++;
}
--
Kees Cook @outflux.net
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Fix potential truncation of mdraid device list
2021-01-16 11:04 [PATCH] Fix potential truncation of mdraid device list Kees Cook
@ 2021-01-19 21:51 ` Petr Vorel
0 siblings, 0 replies; 2+ messages in thread
From: Petr Vorel @ 2021-01-19 21:51 UTC (permalink / raw)
To: The development of GNU GRUB
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++;
> }
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-01-19 21:51 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-16 11:04 [PATCH] Fix potential truncation of mdraid device list Kees Cook
2021-01-19 21:51 ` Petr Vorel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.