linux-kernel - Re: [PATCH 03/10] mpt3sas: Implement device_remove_in

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+RiK67ABpPAX18i6P89cSwNZZNmNHHPAudhb4s3is0x9Zf2uA@mail.gmail.com>
Date:   Tue, 25 Oct 2016 16:06:57 +0530
From:   Suganath Prabu Subramani <suganath-prabu.subramani@...adcom.com>
To:     Hannes Reinecke <hare@...e.de>
Cc:     JBottomley@...allels.com, jejb@...nel.org,
        Christoph Hellwig <hch@...radead.org>,
        martin.petersen@...cle.com, linux-scsi@...r.kernel.org,
        Sathya Prakash <Sathya.Prakash@...adcom.com>,
        Kashyap Desai <kashyap.desai@...adcom.com>,
        Krishnaraddi Mankani <krishnaraddi.mankani@...adcom.com>,
        linux-kernel@...r.kernel.org,
        Chaitra Basappa <chaitra.basappa@...adcom.com>,
        Sreekanth Reddy <sreekanth.reddy@...adcom.com>
Subject: Re: [PATCH 03/10] mpt3sas: Implement device_remove_in_progress check
 in IOCTL path

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@...e.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@...e.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)