Skip to content

Commit

Permalink
nvmet-tcp: fix potential race of tcp socket closing accept_work
Browse files Browse the repository at this point in the history
[ Upstream commit 0fbcfb0 ]

When we accept a TCP connection and allocate an nvmet-tcp queue we should
make sure not to fully establish it or reference it as the connection may
be already closing, which triggers queue release work, which does not
fence against queue establishment.

In order to address such a race, we make sure to check the sk_state and
contain the queue reference to be done underneath the sk_callback_lock
such that the queue release work correctly fences against it.

Fixes: 872d26a ("nvmet-tcp: add NVMe over TCP target driver")
Reported-by: Elad Grupi <elad.grupi@dell.com>
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
sagigrimberg authored and gregkh committed Mar 4, 2021
1 parent 91edfca commit 5f8ab7f
Showing 1 changed file with 18 additions and 10 deletions.
28 changes: 18 additions & 10 deletions drivers/nvme/target/tcp.c
Expand Up @@ -1485,17 +1485,27 @@ static int nvmet_tcp_set_queue_sock(struct nvmet_tcp_queue *queue)
if (inet->rcv_tos > 0)
ip_sock_set_tos(sock->sk, inet->rcv_tos);

ret = 0;
write_lock_bh(&sock->sk->sk_callback_lock);
sock->sk->sk_user_data = queue;
queue->data_ready = sock->sk->sk_data_ready;
sock->sk->sk_data_ready = nvmet_tcp_data_ready;
queue->state_change = sock->sk->sk_state_change;
sock->sk->sk_state_change = nvmet_tcp_state_change;
queue->write_space = sock->sk->sk_write_space;
sock->sk->sk_write_space = nvmet_tcp_write_space;
if (sock->sk->sk_state != TCP_ESTABLISHED) {
/*
* If the socket is already closing, don't even start
* consuming it
*/
ret = -ENOTCONN;
} else {
sock->sk->sk_user_data = queue;
queue->data_ready = sock->sk->sk_data_ready;
sock->sk->sk_data_ready = nvmet_tcp_data_ready;
queue->state_change = sock->sk->sk_state_change;
sock->sk->sk_state_change = nvmet_tcp_state_change;
queue->write_space = sock->sk->sk_write_space;
sock->sk->sk_write_space = nvmet_tcp_write_space;
queue_work_on(queue_cpu(queue), nvmet_tcp_wq, &queue->io_work);
}
write_unlock_bh(&sock->sk->sk_callback_lock);

return 0;
return ret;
}

static int nvmet_tcp_alloc_queue(struct nvmet_tcp_port *port,
Expand Down Expand Up @@ -1543,8 +1553,6 @@ static int nvmet_tcp_alloc_queue(struct nvmet_tcp_port *port,
if (ret)
goto out_destroy_sq;

queue_work_on(queue_cpu(queue), nvmet_tcp_wq, &queue->io_work);

return 0;
out_destroy_sq:
mutex_lock(&nvmet_tcp_queue_mutex);
Expand Down

0 comments on commit 5f8ab7f

Please sign in to comment.