Re: [PATCH 03/10] mpt3sas: Implement device_remove_in_progress check in IOCTL path

[Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>]

Hi Hannes,

Thanks,
>From facts.MaxDevHandle value from firmware, driver will get to know
the max device handle or devices that it can support. Based on that
value only driver will allocate memory for bit mapping.
And driver will not receive any events from controller, if it exceeds
the max devices it can support. So, there is No possibility that
driver can add more devices than bitmap.

Thanks,
Suganath Prabu S



On Tue, Oct 25, 2016 at 3:21 PM, Hannes Reinecke <hare@suse.de> wrote:
> On 10/25/2016 11:19 AM, Suganath Prabu Subramani wrote:
>> Hi Hannes,
>>
>> Please give us little more info on the third comment. It ll help us to
>> understand better and
>> incorporate required changes.
>>
>> Comment is  "Why don't you need to check for the size of the bitmap here?"
>>
>> i have taken care of other two comments in this patch.
>>
>>>       /* check if device is present */
>>> @@ -5467,6 +5482,7 @@ _scsih_add_device(struct MPT3SAS_ADAPTER *ioc, u16 handle, u8 phy_num,
>>>         sas_device = mpt3sas_get_sdev_by_addr(ioc,
>>>                                         sas_address);
>>>         if (sas_device) {
>>> +               clear_bit(handle, ioc->pend_os_device_add);
>>>                 sas_device_put(sas_device);
>>>                 return -1;
>>>         }
>>
>> Why don't you need to check for the size of the bitmap here?
>>
>>
> Thing is, you are using 'ioc->pend_os_device_add' as a bitmap to track
> which devices to add.
> Which in turn means that the overall number of devices you _can_ add is
> restricted by the size of the bitmap.
> But as you're adding devices you (might) increase the number of devices,
> potentially overflowing the bitmap.
> Hence the question: is it possible that you can add _more_ devices than
> the bitmap can hold?
> Or, to put it the other way round: Why don't you need to check the size
> of the bitmap to avoid accessing an invalid bit beyond the end of the mask?
>
> Cheers,
>
> Hannes
> --
> Dr. Hannes Reinecke                Teamlead Storage & Networking
> hare@suse.de                                   +49 911 74053 688
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)