[PATCH] nvme: tcp: avoid race between lock and destroy of queue_lock

[Hannes Reinecke]

On 10/1/24 11:38, Shinichiro Kawasaki wrote:
> On Sep 12, 2024 / 13:28, Shinichiro Kawasaki wrote:
> [...]
>> Having said that, I still think there is a very tiny possibility that the
>> queue_lock gets destroyed between the NVME_TCP_Q_LIVE flag check and the
>> mutex_lock(). That is why I suggested the new flag NVME_TCP_Q_REFERRED.
>> Am I overthinking?
> 
> I thought about this again, now I think the patch I suggested is too much. The
> fix Hannes suggested is much simpler, and I confirmed it avoids the failure that
> I observed. Now I hope this fix to be applied.
> 
> Hannes,
> 
> May I send the fix as a formal patch on behalf of you? The fix will be as
> follows, and I will put your authorship to it.
> 
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 89c44413c593..3e416af2659f 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -2644,10 +2644,11 @@ static int nvme_tcp_get_address(struct nvme_ctrl *ctrl, char *buf, int size)
>   
>   	len = nvmf_get_address(ctrl, buf, size);
>   
> +	if (!test_bit(NVME_TCP_Q_LIVE, &queue->flags))
> +		return len;
> +
>   	mutex_lock(&queue->queue_lock);
>   
> -	if (!test_bit(NVME_TCP_Q_LIVE, &queue->flags))
> -		goto done;
>   	ret = kernel_getsockname(queue->sock, (struct sockaddr *)&src_addr);
>   	if (ret > 0) {
>   		if (len > 0)
> @@ -2655,7 +2656,7 @@ static int nvme_tcp_get_address(struct nvme_ctrl *ctrl, char *buf, int size)
>   		len += scnprintf(buf + len, size - len, "%ssrc_addr=%pISc\n",
>   				(len) ? "," : "", &src_addr);
>   	}
> -done:
> +
>   	mutex_unlock(&queue->queue_lock);
>   
>   	return len;

Sure, you're welcome to send it :-)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare at suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich