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

On 9 April 2017 at 13:27, Simon Glass sjg@chromium.org wrote:

Hi,

On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@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@ti.com

common/scsi.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-)

Reviewed-by: Simon Glass sjg@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

Applied to u-boot-dm, thanks!