[prev in list] [next in list] [prev in thread] [next in thread] 

List:       grub-devel
Subject:    [PATCH] osdep/linux: Fix md array device enumeration
From:       Kees Cook <kees () ubuntu ! com>
Date:       2021-01-17 21:38:44
Message-ID: 20210117213844.GC4996 () outflux ! net
[Download RAW message or body]

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>
---
Actually, here's a better patch which combines the two I sent...
---
 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 001b818fe581..d7b62e702e1f 100644
--- a/grub-core/osdep/linux/getroot.c
+++ b/grub-core/osdep/linux/getroot.c
@@ -135,10 +135,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;
@@ -170,22 +178,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.25.1

-- 
Kees Cook

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[prev in list] [next in list] [prev in thread] [next in thread]