From: Kees Cook <k...@ubuntu.com>
GET_ARRAY_INFO's info.nr_disks does not map to GET_DISK_INFO's
disk.number, which is an internal kernel index. If an array has had drives
added, removed, etc, there may be gaps in GET_DISK_INFO's results. But
since the consumer of devicelist cannot tolerate gaps (it expects to walk
a NULL-terminated list of device name strings), the devicelist index (j)
must be tracked separately from the disk.number index (i). But grub
wants to only examine active (i.e. present and non-failed) disks, so how
many disks are left to be queried must be also separately tracked
(remaining).
Fixes: 49de079bbe1c ("... (grub_util_raid_getmembers): Handle "removed" disks")
Fixes: 2b00217369ac ("... Added support for RAID and LVM")
Fixes: https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1912043
Fixes: https://savannah.gnu.org/bugs/index.php?59887
Signed-off-by: Kees Cook <k...@ubuntu.com>
---
Hi, this is a resend. Petr reviewed an earlier version back in Jan[1],
but given the changes[2] I didn't want to assume that still stood. :)
Regardless, I'd still like to see this merged so I don't have to
trip over this bug again. ;)
[1] https://lists.gnu.org/archive/html/grub-devel/2021-01/msg00040.html
[2] https://lists.gnu.org/archive/html/grub-devel/2021-01/msg00030.html
---
grub-core/osdep/linux/getroot.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/grub-core/osdep/linux/getroot.c b/grub-core/osdep/linux/getroot.c
index cd588588eecf..a359d749fef5 100644
--- a/grub-core/osdep/linux/getroot.c
+++ b/grub-core/osdep/linux/getroot.c
@@ -130,10 +130,18 @@ struct mountinfo_entry
char fstype[ESCAPED_PATH_MAX + 1], device[ESCAPED_PATH_MAX + 1];
};
+/* GET_DISK_INFO nr_disks (total count) does not map to disk.number,
+ which is an internal kernel index. Instead, do what mdadm does
+ and keep scanning until we find enough valid disks. The limit is
+ copied from there, which notes that it is sufficiently high given
+ that the on-disk metadata for v1.x can only support 1920. */
+#define MD_MAX_DISKS 4096
+
static char **
grub_util_raid_getmembers (const char *name, int bootable)
{
int fd, ret, i, j;
+ int remaining;
char **devicelist;
mdu_version_t version;
mdu_array_info_t info;
@@ -165,22 +173,25 @@ grub_util_raid_getmembers (const char *name, int bootable)
devicelist = xcalloc (info.nr_disks + 1, sizeof (char *));
- for (i = 0, j = 0; j < info.nr_disks; i++)
+ remaining = info.nr_disks;
+ for (i = 0, j = 0; i < MD_MAX_DISKS && remaining > 0; 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));
-
+
+ /* Skip empty slot: MD_DISK_REMOVED slots don't count toward nr_disks. */
if (disk.state & (1 << MD_DISK_REMOVED))
continue;
+ remaining--;
- if (disk.state & (1 << MD_DISK_ACTIVE))
- devicelist[j] = grub_find_device (NULL,
- makedev (disk.major, disk.minor));
- else
- devicelist[j] = NULL;
- j++;
+ /* Only examine disks that are actively participating in the array. */
+ if (!(disk.state & (1 << MD_DISK_ACTIVE)))
+ continue;
+
+ devicelist[j++] = grub_find_device (NULL, makedev (disk.major,
+ disk.minor));
}
devicelist[j] = NULL;
--
2.30.2
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
