All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <kees@ubuntu.com>
To: grub-devel@gnu.org
Subject: [PATCH] osdep/linux: Fix md array device enumeration
Date: Sun, 17 Jan 2021 13:38:44 -0800	[thread overview]
Message-ID: <20210117213844.GC4996@outflux.net> (raw)
In-Reply-To: <20210116172728.GB4996@outflux.net>

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


  reply	other threads:[~2021-01-17 21:38 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-16 17:27 [PATCH] Fix md RAID enumeration Kees Cook
2021-01-17 21:38 ` Kees Cook [this message]
  -- strict thread matches above, loose matches on Subject: below --
2021-09-26  2:03 [PATCH] osdep/linux: Fix md array device enumeration kees
2021-10-05 16:38 ` Daniel Kiper
2021-10-06  7:28   ` Petr Vorel
2021-10-07 23:36     ` Kees Cook
2021-10-08  8:25       ` Petr Vorel
2021-10-07 23:34   ` Kees Cook
2023-06-06 16:10 Julian Andres Klode
2023-06-06 16:15 ` Julian Andres Klode
2023-06-06 17:09   ` Daniel Kiper
2023-06-06 17:26     ` Julian Andres Klode
2023-06-06 18:02       ` Kees Cook
2023-06-07 13:39         ` Daniel Kiper
2023-06-07 21:33           ` Kees Cook
2023-06-12 14:24             ` Daniel Kiper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210117213844.GC4996@outflux.net \
    --to=kees@ubuntu.com \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.