Re: [PATCH] osdep/linux: Fix md array device enumeration

[Petr Vorel <pvorel@suse.cz>]

Hi Kees, Daniel,

> On Sat, Sep 25, 2021 at 07:03:35PM -0700, kees@ubuntu.com wrote:
> > From: Kees Cook <kees@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 <pvorel@suse.cz>

Kind regards,
Petr