Hi Kees, Daniel, > On Sat, Sep 25, 2021 at 07:03:35PM -0700, k...@ubuntu.com wrote: > > 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). Kees, thanks a lot for taking care for this. <snip> > > + /* 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)); > I would prefer if you leave original if statement as is and update > grub_find_device() call accordingly... And of course drop else as > needed... :-) I suppose you'll need to send v2 due this, but for now: Reviewed-by: Petr Vorel <pvo...@suse.cz> Kind regards, Petr _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
