[PATCH 2/2] RDMA/rxe: copy completer WQE to kernel buffer before processing
Tristan Madani <[email protected]>
| Newsgroups | org.kernel.vger.stable,org.kernel.vger.linux-rdma |
|---|---|
| Message-ID | <[email protected]> |
From: Tristan Madani <[email protected]> The completer reads WQE fields (dma.num_sge, dma.sge[], dma.cur_sge, wr.opcode, first_psn, last_psn, state, status) directly from the userspace-mapped shared send queue via queue_head(). Userspace can modify these fields between completer reads, causing inconsistent state in do_read() and do_atomic() which call copy_data(). This is the completer-path counterpart to the preceding patch which fixed the requester path. Although rxe_sender() calls the requester and completer sequentially (not concurrently), the requester may have already advanced wqe_index and invalidated its kernel-private copy by the time the completer processes the READ/ATOMIC response, so the completer needs its own copy. Fix by: 1. Adding a kernel-private WQE copy buffer (comp_wqe) to rxe_comp_info 2. Copying the WQE from shared memory in get_wqe() before any processing, with num_sge validation against qp->sq.max_sge 3. Writing back status/state changes to the shared queue entry in do_complete() so CQE generation and retry logic see the correct completion status The flush_send_queue() error path is not modified as it runs during QP teardown (ERR/RESET state) and does not call copy_data(). Fixes: 8700e3e7c485 ("Soft RoCE driver") Cc: [email protected] Signed-off-by: Tristan Madani <[email protected]> --- drivers/infiniband/sw/rxe/rxe_comp.c | 36 +++++++++++++++++++++++++-- drivers/infiniband/sw/rxe/rxe_verbs.h | 5 ++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c index 1390e861bd1d7..0f647721a657e 100644 --- a/drivers/infiniband/sw/rxe/rxe_comp.c +++ b/drivers/infiniband/sw/rxe/rxe_comp.c @@ -142,16 +142,40 @@ static inline enum comp_state get_wqe(struct rxe_qp *qp, struct rxe_send_wqe **wqe_p) { struct rxe_send_wqe *wqe; + unsigned int num_sge; + size_t copy_size; /* we come here whether or not we found a response packet to see if * there are any posted WQEs */ wqe = queue_head(qp->sq.queue, QUEUE_TYPE_FROM_CLIENT); - *wqe_p = wqe; /* no WQE or requester has not started it yet */ - if (!wqe || wqe->state == wqe_state_posted) + if (!wqe || wqe->state == wqe_state_posted) { + *wqe_p = NULL; return pkt ? COMPST_DONE : COMPST_EXIT; + } + + /* Copy WQE from userspace-mapped shared queue to kernel-private + * buffer. Userspace can concurrently modify DMA state (num_sge, + * sge[], cur_sge), leading to inconsistent state in do_read() + * and do_atomic(). This is the completer-path counterpart to + * the requester-path fix. + */ + num_sge = wqe->dma.num_sge; + if (unlikely(num_sge > qp->sq.max_sge)) { + rxe_dbg_qp(qp, "invalid num_sge in send WQE (comp)\n"); + memcpy(&qp->comp.comp_wqe.wqe, wqe, sizeof(*wqe)); + qp->comp.comp_wqe.wqe.status = IB_WC_LOC_LEN_ERR; + qp->comp.shared_wqe = wqe; + *wqe_p = &qp->comp.comp_wqe.wqe; + return COMPST_ERROR; + } + copy_size = sizeof(*wqe) + num_sge * sizeof(struct rxe_sge); + memcpy(&qp->comp.comp_wqe.wqe, wqe, copy_size); + qp->comp.shared_wqe = wqe; + *wqe_p = &qp->comp.comp_wqe.wqe; + wqe = *wqe_p; /* WQE does not require an ack */ if (wqe->state == wqe_state_done) @@ -446,6 +470,14 @@ static void do_complete(struct rxe_qp *qp, struct rxe_send_wqe *wqe) struct rxe_cqe cqe; bool post; + /* Write back status to the shared queue entry so the CQE + * and any retry logic sees the correct completion status. + */ + if (qp->comp.shared_wqe) { + qp->comp.shared_wqe->status = wqe->status; + qp->comp.shared_wqe->state = wqe->state; + } + /* do we need to post a completion */ post = ((qp->sq_sig_type == IB_SIGNAL_ALL_WR) || (wqe->wr.send_flags & IB_SEND_SIGNALED) || diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h index a22dfc6e5ae3c..f1d647ddc1e05 100644 --- a/drivers/infiniband/sw/rxe/rxe_verbs.h +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h @@ -130,6 +130,11 @@ struct rxe_comp_info { int started_retry; u32 retry_cnt; u32 rnr_retry; + struct rxe_send_wqe *shared_wqe; + struct { + struct rxe_send_wqe wqe; + struct ib_sge sge[RXE_MAX_SGE]; + } comp_wqe; }; /* responder states */ -- 2.47.3