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

[Simon Glass]

On 9 April 2017 at 13:27, Simon Glass <sjg at chromium.org> wrote:
> 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

Applied to u-boot-dm, thanks!