Skip to contentSkip to conversation

RDMA/rxe: Fix TOCTOU heap overflow in get_srq_wqe

PATCH Applied Bjoern Doebel 2 participants 2 messages Sep 3 – Sep 6

Subsystems (2)
Stable branch InfiniBand
lore
Share & tools
lore

Bjoern Doebel

author
#0 lore raw

PATCH 1/1 RDMA/rxe: Fix TOCTOU heap overflow in get_srq_wqe

[ Upstream commit 22b8fbded65b8c441b634a185f8da67657df6c50 ]

get_srq_wqe() reads wqe->dma.num_sge from the shared receive queue buffer, which is mapped into userspace. It validates num_sge against max_sge, but then re-reads the same field to calculate the memcpy size. A concurrent userspace thread can modify num_sge between validation and use, causing a heap buffer overflow when copying the WQE into qp->resp.srq_wqe.

Read num_sge into a local variable and use it for both the bounds check and the size calculation.

Fixes 8700e3e7c485 ("Soft RoCE driver")
Link https://patch.msgid.link/r/[email protected] open here
Signed-off-by Tristan Madani <[email protected]>
Reviewed-by Zhu Yanjun <[email protected]>
Signed-off-by Jason Gunthorpe <[email protected]>
Signed-off-by Sasha Levin <[email protected]>

[doebel: cherry-picked from v6.1.178 (3cfa2a3adc51), adjusted for
context]

Signed-off-by Bjoern Doebel <[email protected]>
---
 drivers/infiniband/sw/rxe/rxe_resp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/infiniband/sw/rxe/rxe_resp.c +4 -2
index e7dec84810614..27f24c77993ca 100644
@@ -296,6 +296,7 @@ static enum resp_states get_srq_wqe(struct rxe_qp *qp)
 	struct rxe_recv_wqe *wqe;
 	struct ib_event ev;
 	unsigned int count;
+	unsigned int num_sge;
 	size_t size;
 
 	if (srq->error)

@@ -310,12 +311,13 @@ static enum resp_states get_srq_wqe(struct rxe_qp *qp)
 	}
 
 	/* don't trust user space data */
-	if (unlikely(wqe->dma.num_sge > srq->rq.max_sge)) {
+	num_sge = wqe->dma.num_sge;
+	if (unlikely(num_sge > srq->rq.max_sge)) {
 		spin_unlock_bh(&srq->rq.consumer_lock);
 		pr_warn("%s: invalid num_sge in SRQ entry\n", __func__);
 		return RESPST_ERR_MALFORMED_WQE;
 	}
-	size = sizeof(*wqe) + wqe->dma.num_sge*sizeof(struct rxe_sge);
+	size = sizeof(*wqe) + num_sge * sizeof(struct rxe_sge);
 	memcpy(&qp->resp.srq_wqe, wqe, size);
 
 	qp->resp.wqe = &qp->resp.srq_wqe.wqe;
Signature · 1 line
2.50.1
maintainer Applied
#1 lore raw
Quoted 5 lines

get_srq_wqe() reads wqe->dma.num_sge from the shared receive queue buffer, which is mapped into userspace. It validates num_sge against max_sge, but then re-reads the same field to calculate the memcpy size. A concurrent userspace thread can modify num_sge between validation and use, causing a heap buffer overflow when copying the

Queued for 5.15, thanks.

Signature · 2 lines
Thanks,
Sasha