Re: [PATCH V4] ieee1275/ofdisk: vscsi lun handling on lun len

[Avnish Chouhan <avnish@linux.ibm.com>]

> Message: 3
> Date: Mon, 11 Nov 2024 14:42:55 +0530
> From: Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com>
> To: grub-devel@gnu.org
> Cc: meghanaprakash@in.ibm.com, avnish@linux.vnet.ibm.com,
> 	brking@linux.vnet.ibm.com, mamatha4@linux.vnet.ibm.com,
> 	mchauras@linux.vnet.ibm.com, Mukesh Kumar Chaurasiya
> 	<mchauras@linux.ibm.com>
> Subject: [PATCH V4] ieee1275/ofdisk: vscsi lun handling on lun len
> Message-ID: <20241111091254.775590-2-mchauras@linux.ibm.com>
> 
> 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 <mchauras@linux.ibm.com>
> ---
>  grub-core/disk/ieee1275/ofdisk.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/grub-core/disk/ieee1275/ofdisk.c 
> b/grub-core/disk/ieee1275/ofdisk.c
> index c6cba0c8a..b446bb1e7 100644
> --- a/grub-core/disk/ieee1275/ofdisk.c
> +++ b/grub-core/disk/ieee1275/ofdisk.c
> @@ -43,6 +43,12 @@ struct ofdisk_hash_ent
>    struct ofdisk_hash_ent *next;
>  };
> 
> +struct lun_buf
> +{
> +  grub_uint64_t buf_addr;
> +  grub_uint64_t lun_count;
> +};
> +
>  static grub_err_t
>  grub_ofdisk_get_block_size (const char *device, grub_uint32_t 
> *block_size,
>  			    struct ofdisk_hash_ent *op);
> @@ -222,8 +228,9 @@ dev_iterate (const struct grub_ieee1275_devalias 
> *alias)
>  	grub_ieee1275_cell_t table;
>        }
>        args;
> +      struct lun_buf *tbl;
>        char *buf, *bufptr;
> -      unsigned i;
> +      unsigned int i, j;
> 
>        if (grub_ieee1275_open (alias->path, &ihandle))
>  	return;
> @@ -248,17 +255,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;
> --
> 2.47.0
> 
> 
> 
> 
> ------------------------------


Reviewed-by: Avnish Chouhan <avnish@linux.ibm.com>

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel