[PATCH 2/2] nvmet: fix a race condition between release_queue and io_work

[Sagi Grimberg]

>> So this means we still get data from the network when
>> we shouldn't. Maybe we are simply missing a kernel_sock_shutdown
>> for SHUT_RD?
> 
> Hmm, right, kernel_sock_shutdown(queue->sock) is executed in
> nvmet_tcp_delete_ctrl() and sock_release(queue->sock) is called
> in nvmet_tcp_release_queue_work(), so there could be a race here.
> 
> I will try to move kernel_sock_shutdown(queue->sock) in nvmet_tcp_release_queue_work()
> and test it.

Not move, but perhaps add another SHUT_RD call.

> 
>>
>>>
>>>>
>>>>> * Fix this bug by preventing io_work from being enqueued when
>>>>> sk_user_data is NULL (it means that the queue is going to be deleted)
>>>>
>>>> This is triggered from the completion path, where the commands
>>>> are not in a state where they are still fetching data from the
>>>> host. How does this prevent the crash?
>>>
>>> io_work is also triggered every time a nvmet_req_init() fails and when
>>> nvmet_sq_destroy() is called, I am not really sure about the state
>>> of the commands in those cases.
>>
>> But that is from the workqueue context - which means that
>> cancel_work_sync should prevent it right?
> 
> 
> But nvmet_sq_destroy() is called from the release_work context,

Yes

> we call cancel_work_sync() immediately after but we can't be sure
> that the work will be canceled, io_work might have started already and
> cancel_work_sync() will block until io_work ends its job, right?

Right, after the call to cancel_work_sync we will know that io_work
is not running. Note that it can run as a result of a backend completion
but that is ok and we do want to let it run and return completion to the
host, but the socket should already be shut down for recv, so we cannot
get any other byte from the network.