On Mon, Jun 10, 2024 at 11:29:56AM +0530, Mukesh Kumar Chaurasiya wrote:
> The information about "vscsi-report-luns" data is a list of disk details
> with pairs of memory addresses and lengths.
>
> 8 bytes 8 bytes
> lun-addr ---> ------------------------ 8 bytes
> ^ | buf-addr | lun-count| --------> -------------
> | ------------------------ | lun |
> | | buf-addr | lun-count| ----| -------------
> "len" ------------------------ | | ... |
> | | ... | | -------------
> | ------------------------ | | lun |
> | | buf-addr | lun-count| | -------------
> V ------------------------ |
> |---> -------------
> | lun |
> -------------
> | ... |
> -------------
> | lun |
> -------------
> The way the expression (args.table + 4 + 8 * i) is used is incorrect and
> can be confusing. The list of LUNs doesn't end with NULL, indicated by
> while (*ptr). Usually, this loop doesn't process any LUNs because it ends
> before checking any as first reported LUN is likely to be 0. The list of
> LUNs ends based on its length, not by a NULL value.
>
> Signed-off-by: Mukesh Kumar Chaurasiya <mchau...@linux.ibm.com>
> ---
> grub-core/disk/ieee1275/ofdisk.c | 27 ++++++++++++++++-----------
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/grub-core/disk/ieee1275/ofdisk.c
> b/grub-core/disk/ieee1275/ofdisk.c
> index c6cba0c8a..1618544a8 100644
> --- a/grub-core/disk/ieee1275/ofdisk.c
> +++ b/grub-core/disk/ieee1275/ofdisk.c
> @@ -222,8 +222,12 @@ dev_iterate (const struct grub_ieee1275_devalias *alias)
> grub_ieee1275_cell_t table;
> }
> args;
> + struct lun_buf {
> + grub_uint64_t *buf_addr;
Again, this is wrong taking into account diagram above. I think it
should be "grub_uint64_t buf_addr". Then you should convert it to the
pointer. Though please be cautious on the 32-bit platforms...
> + grub_uint64_t lun_count;
> + } *tbl;
> char *buf, *bufptr;
> - unsigned i;
> + unsigned int i, j;
>
> if (grub_ieee1275_open (alias->path, &ihandle))
> return;
> @@ -248,17 +252,18 @@ dev_iterate (const struct grub_ieee1275_devalias *alias)
> return;
> bufptr = grub_stpcpy (buf, alias->path);
>
> + tbl = (struct lun_len *) args.table;
> for (i = 0; i < args.nentries; i++)
> - {
> - grub_uint64_t *ptr;
> -
> - ptr = *(grub_uint64_t **) (args.table + 4 + 8 * i);
> - while (*ptr)
> - {
> - grub_snprintf (bufptr, 32, "/disk@%" PRIxGRUB_UINT64_T, *ptr++);
> - dev_iterate_real (buf, buf);
> - }
> - }
> + {
> + grub_uint64_t *ptr;
> +
> + ptr = (grub_uint64_t *)(grub_addr_t) tbl[i].buf_addr;
> + for (j = 0; j < tbl[i].lun_count; j++)
> + {
> + grub_snprintf (bufptr, 32, "/disk@%" PRIxGRUB_UINT64_T, *ptr++);
> + dev_iterate_real (buf, buf);
> + }
> + }
> grub_ieee1275_close (ihandle);
> grub_free (buf);
> return;
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
