RDMA/rxe: Fix TOCTOU heap overflow in get_srq_wqe

Upstream commit 22b8fbded65b ("RDMA/rxe: Fix TOCTOU heap overflow in get_srq_wqe") fixes a heap buffer overflow in the rxe responder.

The fix does not apply to 5.10.y as-is because get_srq_wqe() there predates three earlier cleanups. This series therefore backports those prerequisites from 5.14 first, and then the actual fix:

1/4 ec0fa2445c18 RDMA/rxe: Fix over copying in get_srq_wqe
(context adjusted for 5.10)
2/4 36941dfe0e8c RDMA/rxe: Missing unlock on error in get_srq_wqe()
(clean cherry-pick)
3/4 e2a05339fa11 RDMA/rxe: Use the correct size of wqe when processing SRQ
(clean cherry-pick)
4/4 22b8fbded65b RDMA/rxe: Fix TOCTOU heap overflow in get_srq_wqe
(cherry-picked from the v6.1.178 backport 3cfa2a3adc51, context
adjusted)

Tested in the Amazon Linux kernel test suite.

Bjoern

Bob Pearson (2):
RDMA/rxe: Fix over copying in get_srq_wqe RDMA/rxe: Use the correct size of wqe when processing SRQ

Dan Carpenter (1):
RDMA/rxe: Missing unlock on error in get_srq_wqe()

Tristan Madani (1):
RDMA/rxe: Fix TOCTOU heap overflow in get_srq_wqe

 drivers/infiniband/sw/rxe/rxe_resp.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

2.50.1

From: Bob Pearson <[email protected]>

[ Upstream commit ec0fa2445c18ec49a0b7ee0aaa82d1ec00968fc9 ]

Currently get_srq_wqe() in rxe_resp.c copies the maximum possible number of bytes from the wqe into the QPs copy of the SRQ wqe. This is usually extra work and risks reading past the end of the SRQ circular buffer if the SRQ is configured with less than the maximum possible number of SGEs.

Check the number of SGEs is not too large.
Compute the actual number of bytes in the WR and copy only those.

[doebel: context adjustment from upstream patch to 5.10. needed as a prerequisite
for "RDMA/rxe: Fix TOCTOU heap overflow in get_srq_wqe" ]

---
 drivers/infiniband/sw/rxe/rxe_resp.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

index 83c03212099a2..38e6535196849 100644
@@ -292,6 +292,7 @@ static enum resp_states get_srq_wqe(struct rxe_qp *qp)
 	struct rxe_queue *q = srq->rq.queue;
 	struct rxe_recv_wqe *wqe;
 	struct ib_event ev;
+	size_t size;
 
 	if (srq->error)
 		return RESPST_ERR_RNR;

@@ -304,8 +305,13 @@ static enum resp_states get_srq_wqe(struct rxe_qp *qp)
 		return RESPST_ERR_RNR;
 	}
 
-	/* note kernel and user space recv wqes have same size */
-	memcpy(&qp->resp.srq_wqe, wqe, sizeof(qp->resp.srq_wqe));
+	/* don't trust user space data */
+	if (unlikely(wqe->dma.num_sge > srq->rq.max_sge)) {
+		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);
+	memcpy(&qp->resp.srq_wqe, wqe, size);
 
 	qp->resp.wqe = &qp->resp.srq_wqe.wqe;
 	advance_consumer(q);

2.50.1

From: Dan Carpenter <[email protected]>

[ Upstream commit 36941dfe0e8c3e2da7851b9648fd74bd3a3e78ce ]

This error path needs to unlock before returning.

[doebel: Clean cherry-pick. Needed as a prerequisiste for
"RDMA/rxe: Fix TOCTOU heap overflow in get_srq_wqe" ]

---
 drivers/infiniband/sw/rxe/rxe_resp.c | 1 +
 1 file changed, 1 insertion(+)

index 38e6535196849..207889332b963 100644
@@ -307,6 +307,7 @@ 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)) {
+		spin_unlock_bh(&srq->rq.consumer_lock);
 		pr_warn("%s: invalid num_sge in SRQ entry\n", __func__);
 		return RESPST_ERR_MALFORMED_WQE;
 	}

2.50.1

From: Bob Pearson <[email protected]>

[ Upstream commit e2a05339fa1188b6b37540f4611893ac4c534fa2 ]

The memcpy() that copies a WQE from a SRQ the QP uses an incorrect size. The size should have been the size of the rxe_send_wqe struct not the size of a pointer to it. The result is that IO operations using a SRQ on the responder side will fail.

[doebel: Clean cherry-pick. Needed as a prerequisite for
"RDMA/rxe: Fix TOCTOU heap overflow in get_srq_wqe" ]

---
 drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 207889332b963..55b5146424e61 100644
@@ -311,7 +311,7 @@ static enum resp_states get_srq_wqe(struct rxe_qp *qp)
 		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) + wqe->dma.num_sge*sizeof(struct rxe_sge);
 	memcpy(&qp->resp.srq_wqe, wqe, size);
 
 	qp->resp.wqe = &qp->resp.srq_wqe.wqe;

2.50.1

From: Tristan Madani <[email protected]>

[ 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.

---
 drivers/infiniband/sw/rxe/rxe_resp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index 55b5146424e61..d22d95f5e2e3c 100644
@@ -292,6 +292,7 @@ static enum resp_states get_srq_wqe(struct rxe_qp *qp)
 	struct rxe_queue *q = srq->rq.queue;
 	struct rxe_recv_wqe *wqe;
 	struct ib_event ev;
+	unsigned int num_sge;
 	size_t size;
 
 	if (srq->error)

@@ -306,12 +307,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;

2.50.1

Queued the series for 5.10, thanks.

Thanks,
Sasha