mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
nvme-tcp: fix NULL queue->ctrl dereference on reconnect
Since commit61b0ed3be8("nvme-tcp: parallelize I/O queue allocation and startup"), a reconnect that gets more I/O queues than before grows the tag set before the new queues are allocated. queue->ctrl is only set by nvme_tcp_alloc_queue(), so I/O submitted in between reaches nvme_tcp_queue_rq() on a queue that was never allocated and dereferences a NULL queue->ctrl: BUG: kernel NULL pointer dereference, address: 00000000000002ac RIP: 0010:nvme_tcp_queue_rq+0x33/0x3b0 Add nvme_tcp_init_queue() for the queue fields that do not depend on the connection and call it for every queue when allocating the controller. Such requests are then requeued like on any other queue that is not live. send_mutex and io_work move there as well, and nvme_tcp_exit_queue() destroys the helper's mutexes when the controller is freed. Fixes:61b0ed3be8("nvme-tcp: parallelize I/O queue allocation and startup") Suggested-by: Keith Busch <kbusch@kernel.org> Reviewed-by: Nilay Shroff <nilay@linux.ibm.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Hannes Reinecke <hare@kernel.org> Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com> [kbusch: dropped the comment above nvme_tcp_init_queue()] Signed-off-by: Keith Busch <kbusch@kernel.org>
This commit is contained in:
parent
2eeeaa9c1f
commit
e35557e41a
|
|
@ -1566,7 +1566,6 @@ static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
|
|||
memalloc_noio_restore(noio_flag);
|
||||
|
||||
kfree(queue->pdu);
|
||||
mutex_destroy(&queue->send_mutex);
|
||||
|
||||
#ifdef CONFIG_DEBUG_LOCK_ALLOC
|
||||
lockdep_unregister_key(&queue->nvme_tcp_sk_key);
|
||||
|
|
@ -1994,6 +1993,28 @@ static int nvme_tcp_start_tls(struct nvme_ctrl *nctrl,
|
|||
return ret;
|
||||
}
|
||||
|
||||
static void nvme_tcp_init_queue(struct nvme_tcp_ctrl *ctrl, int qid)
|
||||
{
|
||||
struct nvme_tcp_queue *queue = &ctrl->queues[qid];
|
||||
|
||||
queue->ctrl = ctrl;
|
||||
mutex_init(&queue->queue_lock);
|
||||
mutex_init(&queue->send_mutex);
|
||||
mutex_init(&queue->pf_cache_lock);
|
||||
INIT_WORK(&queue->io_work, nvme_tcp_io_work);
|
||||
queue->hdr_digest = ctrl->ctrl.opts->hdr_digest;
|
||||
queue->data_digest = ctrl->ctrl.opts->data_digest;
|
||||
}
|
||||
|
||||
static void nvme_tcp_exit_queue(struct nvme_tcp_ctrl *ctrl, int qid)
|
||||
{
|
||||
struct nvme_tcp_queue *queue = &ctrl->queues[qid];
|
||||
|
||||
mutex_destroy(&queue->queue_lock);
|
||||
mutex_destroy(&queue->send_mutex);
|
||||
mutex_destroy(&queue->pf_cache_lock);
|
||||
}
|
||||
|
||||
static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
|
||||
key_serial_t pskid)
|
||||
{
|
||||
|
|
@ -2002,11 +2023,8 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
|
|||
int ret, rcv_pdu_size;
|
||||
struct file *sock_file;
|
||||
|
||||
queue->ctrl = ctrl;
|
||||
init_llist_head(&queue->req_list);
|
||||
INIT_LIST_HEAD(&queue->send_list);
|
||||
mutex_init(&queue->send_mutex);
|
||||
INIT_WORK(&queue->io_work, nvme_tcp_io_work);
|
||||
|
||||
if (qid > 0)
|
||||
queue->cmnd_capsule_len = nctrl->ioccsz * 16;
|
||||
|
|
@ -2020,14 +2038,12 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
|
|||
if (ret) {
|
||||
dev_err(nctrl->device,
|
||||
"failed to create socket: %d\n", ret);
|
||||
goto err_destroy_mutex;
|
||||
return ret;
|
||||
}
|
||||
|
||||
sock_file = sock_alloc_file(queue->sock, O_CLOEXEC, NULL);
|
||||
if (IS_ERR(sock_file)) {
|
||||
ret = PTR_ERR(sock_file);
|
||||
goto err_destroy_mutex;
|
||||
}
|
||||
if (IS_ERR(sock_file))
|
||||
return PTR_ERR(sock_file);
|
||||
|
||||
sk_net_refcnt_upgrade(queue->sock->sk);
|
||||
|
||||
|
|
@ -2095,8 +2111,6 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
|
|||
}
|
||||
}
|
||||
|
||||
queue->data_digest = nctrl->opts->data_digest;
|
||||
|
||||
rcv_pdu_size = sizeof(struct nvme_tcp_rsp_pdu) +
|
||||
nvme_tcp_hdgst_len(queue);
|
||||
queue->pdu = kmalloc(rcv_pdu_size, GFP_KERNEL);
|
||||
|
|
@ -2143,8 +2157,6 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
|
|||
lockdep_unregister_key(&queue->nvme_tcp_sk_key);
|
||||
lockdep_unregister_key(&queue->nvme_tcp_slock_key);
|
||||
#endif
|
||||
err_destroy_mutex:
|
||||
mutex_destroy(&queue->send_mutex);
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
|
@ -2846,7 +2858,7 @@ static void nvme_tcp_free_ctrl(struct nvme_ctrl *nctrl)
|
|||
free_ctrl:
|
||||
put_net(ctrl->net);
|
||||
for (i = 0; i < nr_queues; i++)
|
||||
mutex_destroy(&ctrl->queues[i].queue_lock);
|
||||
nvme_tcp_exit_queue(ctrl, i);
|
||||
kfree(ctrl->queues);
|
||||
kfree(ctrl);
|
||||
}
|
||||
|
|
@ -3245,21 +3257,8 @@ static struct nvme_tcp_ctrl *nvme_tcp_alloc_ctrl(struct device *dev,
|
|||
goto out_free_ctrl;
|
||||
}
|
||||
|
||||
/*
|
||||
* The I/O tag set is allocated before the I/O queues, and initializing
|
||||
* its requests already allocates the PDUs from the page frag caches of
|
||||
* the queues, sized by their header digest setting, so set up the
|
||||
* locks and hdr_digest here.
|
||||
*
|
||||
* The queue locks are owned by the controller, not the queues:
|
||||
* they outlive the per-queue connect/disconnect cycles so that
|
||||
* sysfs readers can never race their destruction.
|
||||
*/
|
||||
for (i = 0; i < ctrl->ctrl.queue_count; i++) {
|
||||
mutex_init(&ctrl->queues[i].pf_cache_lock);
|
||||
ctrl->queues[i].hdr_digest = opts->hdr_digest;
|
||||
mutex_init(&ctrl->queues[i].queue_lock);
|
||||
}
|
||||
for (i = 0; i < ctrl->ctrl.queue_count; i++)
|
||||
nvme_tcp_init_queue(ctrl, i);
|
||||
|
||||
ctrl->net = get_net(current->nsproxy->net_ns);
|
||||
ret = nvme_init_ctrl(&ctrl->ctrl, dev, &nvme_tcp_ctrl_ops, 0);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user