Hi Kees, > Hi Petr,
> On Wed, Oct 06, 2021 at 09:28:32AM +0200, Petr Vorel wrote: > > 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. > You bet! I seem to find some amazing corner cases. ;) Yep, but both of your bug reports are really worth of fixing. > > <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> > Thanks! I've included your reviewed-by in the v2; hopefully that's okay > as the changes were just stylistic. Sure, changes in v2 are really minor + already asked to do by Daniel. Thanks! Kind regards, Petr > -Kees _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
