[U-Boot] [PATCH v2 09/10] dm: scsi: fix divide-by-0 error in scsi_scan()

[Simon Glass]

Hi,

On 7 April 2017 at 05:42, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
> With DM_SCSI enabled, blk_create_devicef() is called with blkz = 0, leading
> to a divide-by-0 exception.
> scsi_detect_dev() can be used to get the required parameters (block size
> and number of blocks) from the drive before calling blk_create_devicef().
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
> ---
>  common/scsi.c | 35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

Please see below.

>
> diff --git a/common/scsi.c b/common/scsi.c
> index 972ef338..d37222c 100644
> --- a/common/scsi.c
> +++ b/common/scsi.c
> @@ -580,9 +580,19 @@ int scsi_scan(int mode)
>                         for (lun = 0; lun < plat->max_lun; lun++) {
>                                 struct udevice *bdev; /* block device */
>                                 /* block device description */
> +                               struct blk_desc _bd;
>                                 struct blk_desc *bdesc;
>                                 char str[10];
>
> +                               scsi_init_dev_desc_priv(&_bd);
> +                               ret = scsi_detect_dev(i, lun, &_bd);
> +                               if (ret)
> +                                       /*
> +                                        * no device detected?
> +                                        * check the next lun.
> +                                        */
> +                                       continue;
> +
>                                 /*
>                                  * Create only one block device and do detection
>                                  * to make sure that there won't be a lot of
> @@ -590,20 +600,27 @@ int scsi_scan(int mode)
>                                  */
>                                 snprintf(str, sizeof(str), "id%dlun%d", i, lun);
>                                 ret = blk_create_devicef(dev, "scsi_blk",
> -                                                         str, IF_TYPE_SCSI,
> -                                                         -1, 0, 0, &bdev);
> +                                               str, IF_TYPE_SCSI,
> +                                               -1,
> +                                               _bd.blksz,
> +                                               _bd.blksz * _bd.lba,
> +                                               &bdev);
>                                 if (ret) {
>                                         debug("Can't create device\n");
>                                         return ret;
>                                 }
> -                               bdesc = dev_get_uclass_platdata(bdev);
>
> -                               scsi_init_dev_desc_priv(bdesc);
> -                               ret = scsi_detect_dev(i, lun, bdesc);
> -                               if (ret) {
> -                                       device_unbind(bdev);
> -                                       continue;
> -                               }
> +                               bdesc = dev_get_uclass_platdata(bdev);
> +                               bdesc->target = i;
> +                               bdesc->lun = lun;
> +                               bdesc->removable = _bd.removable;
> +                               bdesc->type = _bd.type;
> +                               memcpy(&bdesc->vendor, &_bd.vendor,
> +                                      sizeof(_bd.vendor));
> +                               memcpy(&bdesc->product, &_bd.product,
> +                                      sizeof(_bd.product));
> +                               memcpy(&bdesc->revision, &_bd.revision,
> +                                      sizeof(_bd.revision));

Can you please move this block (inside the double for loops) into a
separate function? It is getting too long. A follow-up patch is fine
since you have already done this.

>                                 part_init(bdesc);
>
>                                 if (mode == 1) {
> --
> 1.9.1
>

Regards,
Simon