On 11/11/19 5:07 PM, Bart Van Assche wrote:
> On 11/11/19 2:45 AM, Hannes Reinecke wrote:
>> During rescanning the device might already have been removed, so
>> we should drop the BUG_ON and just ignore the non-existing device.
>>
>> Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
>> ---
>> drivers/scsi/device_handler/scsi_dh_rdac.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c
>> b/drivers/scsi/device_handler/scsi_dh_rdac.c
>> index 5efc959493ec..33a71df5ee59 100644
>> --- a/drivers/scsi/device_handler/scsi_dh_rdac.c
>> +++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
>> @@ -424,8 +424,8 @@ static int check_ownership(struct scsi_device
>> *sdev, struct rdac_dh_data *h)
>> rcu_read_lock();
>> list_for_each_entry_rcu(tmp, &h->ctlr->dh_list, node) {
>> /* h->sdev should always be valid */
>> - BUG_ON(!tmp->sdev);
>> - tmp->sdev->access_state = access_state;
>> + if (tmp->sdev) {
>> + tmp->sdev->access_state = access_state;
>> }
>> rcu_read_unlock();
>> err = SCSI_DH_OK;
>
> Since rdac_bus_detach() may clear h->sdev concurrently with the above
> code I think READ_ONCE() is needed to make the above code safe.
>
> Has it been considered to change the kfree() call in rdac_bus_detach()
> into a kfree_rcu() call? I think otherwise the above
> list_for_each_entry_rcu() loop may trigger a use-after-free.
>
Good point. I'll have a look.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 247165 (AG München), GF: Felix Imendörffer
