Re: [PATCH 1/1] vhost-scsi: Fix handling of multiple calls to vhost_scsi_set_endpoint

[Mike Christie <michael.christie@oracle.com>]

On 1/29/25 10:36 AM, Stefano Garzarella wrote:
>> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
>> index 9a4cbdc607fa..6bb64f3be7db 100644
>> --- a/drivers/vhost/scsi.c
>> +++ b/drivers/vhost/scsi.c
>> @@ -1828,14 +1828,19 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
>>         }
>>     }
>>
>> +    if (vs->vs_tpg) {
>> +        pr_err("vhost-scsi endpoint already set for %s.\n",
>> +               vs->vs_vhost_wwpn);
>> +        ret = -EEXIST;
>> +        goto out;
>> +    }
>> +
>>     len = sizeof(vs_tpg[0]) * VHOST_SCSI_MAX_TARGET;
>>     vs_tpg = kzalloc(len, GFP_KERNEL);
>>     if (!vs_tpg) {
>>         ret = -ENOMEM;
>>         goto out;
>>     }
>> -    if (vs->vs_tpg)
>> -        memcpy(vs_tpg, vs->vs_tpg, len);
>>
>>     mutex_lock(&vhost_scsi_mutex);
>>     list_for_each_entry(tpg, &vhost_scsi_list, tv_tpg_list) {
>> @@ -1851,12 +1856,6 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
>>         tv_tport = tpg->tport;
>>
>>         if (!strcmp(tv_tport->tport_name, t->vhost_wwpn)) {
>> -            if (vs->vs_tpg && vs->vs_tpg[tpg->tport_tpgt]) {
>> -                mutex_unlock(&tpg->tv_tpg_mutex);
>> -                mutex_unlock(&vhost_scsi_mutex);
>> -                ret = -EEXIST;
>> -                goto undepend;
>> -            }
>>             /*
>>              * In order to ensure individual vhost-scsi configfs
>>              * groups cannot be removed while in use by vhost ioctl,
>> @@ -1903,7 +1902,8 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
>>         }
>>         ret = 0;
>>     } else {
>> -        ret = -EEXIST;
>> +        ret = -ENODEV;
>> +        goto free_tpg;
>>     }
> 
> After this block there this code:
> 
>>
>>     /*
>      * Act as synchronize_rcu to make sure access to
>      * old vs->vs_tpg is finished.
>      */
>     vhost_scsi_flush(vs);
>     kfree(vs->vs_tpg);
>     vs->vs_tpg = vs_tpg;
>     goto out;
> 
> Should we adapt also that code, removing the kfree() and updating the comment?
> 
Yeah, I think I should. Will resend.

I think I originally was trying to make the patch smaller to make it easier
to backport but seeing it again I think it makes more sense to do the cleanup
in the same patch.