[PATCH] osdep/linux: Fix md array device enumeration

[kees]

From: Kees Cook <kees@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 <kees@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