From e1dd85a5b73e36f2642c5fe3769415368f3c7541 Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Tue, 29 Jan 2019 16:10:31 -0700 Subject: [PATCH] nvmf: Don't increment current_recv_depth for dummy RECV When a connection goes to close and has no I/O outstanding, the current_recv_depth was being decremented beyond 0 and rolling over. If the poll group then finds a successful receive completion on the next poll (for a command that arrived prior to starting the disconnect but hadn't been processed yet), it would trip the max queue depth check added recently and start another disconnect process. If only one command arrives in this window, everything actually works out ok. However, if there are two receive completions sitting in the completion queue after the disconnect process is started, the first one does the double disconnect and the second one does another disconnect which ends up dereferencing a null pointer. Since there is always a special reserved slot for the dummy recv, don't do decrements or increments of the current_recv_depth for the dummy recv. This allows the code to still enforce the actual max_queue_depth on recvs without underflowing or overflowing the counter. Change-Id: I56c95b2424e956a3b007b25c50cbf47262245b8f Signed-off-by: Ben Walker Reviewed-on: https://review.gerrithub.io/c/442642 Tested-by: SPDK CI Jenkins Reviewed-by: Seth Howell Reviewed-by: Jim Harris --- lib/nvmf/rdma.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/nvmf/rdma.c b/lib/nvmf/rdma.c index d74894058..9ffbcb9b3 100644 --- a/lib/nvmf/rdma.c +++ b/lib/nvmf/rdma.c @@ -871,6 +871,7 @@ spdk_nvmf_rdma_qpair_initialize(struct spdk_nvmf_qpair *qpair) rdma_recv->wr.sg_list = rdma_recv->sgl; rc = ibv_post_recv(rqpair->cm_id->qp, &rdma_recv->wr, &bad_wr); + assert(rqpair->current_recv_depth > 0); rqpair->current_recv_depth--; if (rc) { SPDK_ERRLOG("Unable to post capsule for RDMA RECV\n"); @@ -878,6 +879,7 @@ spdk_nvmf_rdma_qpair_initialize(struct spdk_nvmf_qpair *qpair) return -1; } } + assert(rqpair->current_recv_depth == 0); for (i = 0; i < rqpair->max_queue_depth; i++) { rdma_req = &rqpair->reqs[i]; @@ -980,6 +982,7 @@ request_transfer_out(struct spdk_nvmf_request *req, int *data_posted) return rc; } rdma_req->recv = NULL; + assert(rqpair->current_recv_depth > 0); rqpair->current_recv_depth--; /* Build the response which consists of an optional @@ -2754,7 +2757,6 @@ spdk_nvmf_rdma_close_qpair(struct spdk_nvmf_qpair *qpair) assert(false); return; } - rqpair->current_recv_depth--; rqpair->drain_send_wr.type = RDMA_WR_TYPE_DRAIN_SEND; send_wr.wr_id = (uintptr_t)&rqpair->drain_send_wr; @@ -2856,7 +2858,7 @@ spdk_nvmf_rdma_poller_poll(struct spdk_nvmf_rdma_transport *rtransport, assert(rqpair->disconnect_flags & RDMA_QP_DISCONNECTING); SPDK_DEBUGLOG(SPDK_LOG_RDMA, "Drained QP RECV %u (%p)\n", rqpair->qpair.qid, rqpair); rqpair->disconnect_flags |= RDMA_QP_RECV_DRAINED; - rqpair->current_recv_depth++; + assert(rqpair->current_recv_depth == rqpair->max_queue_depth); /* Don't worry about responding to recv overflow, we are disconnecting anyways */ if (rqpair->disconnect_flags & RDMA_QP_SEND_DRAINED) { spdk_nvmf_rdma_qpair_process_pending(rtransport, rqpair, true);