From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1l1Fl7-0005v5-NM for mharc-grub-devel@gnu.org; Sun, 17 Jan 2021 16:38:53 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:52466) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1l1Fl6-0005tN-1O for grub-devel@gnu.org; Sun, 17 Jan 2021 16:38:52 -0500 Received: from mta.outflux.net ([2001:19d0:2:6:c0de:0:736d:7471]:57779 helo=smtp.outflux.net) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1l1Fl3-000878-HB for grub-devel@gnu.org; Sun, 17 Jan 2021 16:38:51 -0500 Received: from www.outflux.net (serenity.outflux.net [10.2.0.2]) by vinyl.outflux.net (8.15.2/8.15.2/Debian-10) with ESMTP id 10HLcioG005395 for ; Sun, 17 Jan 2021 13:38:44 -0800 Received: by www.outflux.net (Postfix, from userid 501) id 41510612EF; Sun, 17 Jan 2021 13:38:44 -0800 (PST) Date: Sun, 17 Jan 2021 13:38:44 -0800 From: Kees Cook To: grub-devel@gnu.org Subject: [PATCH] osdep/linux: Fix md array device enumeration Message-ID: <20210117213844.GC4996@outflux.net> References: <20210116172728.GB4996@outflux.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210116172728.GB4996@outflux.net> Organization: Ubuntu X-MIMEDefang-Filter: outflux$Revision: 1.316 $ X-HELO: www.outflux.net X-Scanned-By: MIMEDefang 2.83 Received-SPF: none client-ip=2001:19d0:2:6:c0de:0:736d:7471; envelope-from=kees@ubuntu.com; helo=smtp.outflux.net X-Spam_score_int: 7 X-Spam_score: 0.7 X-Spam_bar: / X-Spam_report: (0.7 / 5.0 requ) BAYES_00=-1.9, FORGED_SPF_HELO=2.199, KHOP_HELO_FCRDNS=0.4, SPF_HELO_PASS=-0.001, SPF_NONE=0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 17 Jan 2021 21:38:52 -0000 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 --- 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