[for-next,v2] Subject: RDMA/rxe: Handle zero length rdma

Commit Message

Bob Pearson Jan. 27, 2023, 8:56 p.m. UTC

Currently the rxe driver does not handle all cases of zero length
rdma operations correctly. The client does not have to provide an
rkey for zero length RDMA operations so the rkey provided may be
invalid and should not be used to lookup an mr.

This patch corrects the driver to ignore the provided rkey if the
reth length is zero and make sure to set the mr to NULL. In read_reply()
if length is zero rxe_recheck_mr() is not called. Warnings are added in
the routines in rxe_mr.c to catch NULL MRs when the length is non-zero.

Signed-off-by: Bob Pearson <[e-mail]>
---
v2:
  Rebased to current for-next.
  Cleaned up description to be a little more accurate.

 drivers/infiniband/sw/rxe/rxe_resp.c | 43 ++++++++++++++++------------
 1 file changed, 24 insertions(+), 19 deletions(-)


base-commit: ff33dcb55ca3860092ebb98ad01211159842d566

Patch

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 21b2af948662..b13ae98400c1 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -900,10 +900,11 @@  static enum resp_states read_reply(struct rxe_qp *qp,
 	}
 
 	if (res->state == rdatm_res_state_new) {
-		if (qp->resp.length == 0) {
-			mr = NULL;
-			qp->resp.mr = NULL;
-		} else if (!res->replay) {
+		if (!res->replay || qp->resp.length == 0) {
+			/* if length == 0 mr will be NULL (is ok)
+			 * otherwise qp->resp.mr holds a ref on mr
+			 * which we transfer to mr and drop below.
+			 */
 			mr = qp->resp.mr;
 			qp->resp.mr = NULL;
 		} else {
@@ -917,13 +918,13 @@  static enum resp_states read_reply(struct rxe_qp *qp,
 		else
 			opcode = IB_OPCODE_RC_RDMA_READ_RESPONSE_FIRST;
 	} else {
-		if (qp->resp.length == 0) {
-			mr = NULL;
-		} else {
-			mr = rxe_recheck_mr(qp, res->read.rkey);
-			if (!mr)
-				return RESPST_ERR_RKEY_VIOLATION;
-		}
+		/* re-lookup mr from rkey on all later packets.
+		 * length will be non-zero. This can fail if someone
+		 * modifies or destroys the mr since the first packet.
+		 */
+		mr = rxe_recheck_mr(qp, res->read.rkey);
+		if (!mr)
+			return RESPST_ERR_RKEY_VIOLATION;
 
 		if (res->read.resid > mtu)
 			opcode = IB_OPCODE_RC_RDMA_READ_RESPONSE_MIDDLE;
@@ -938,18 +939,16 @@  static enum resp_states read_reply(struct rxe_qp *qp,
 	skb = prepare_ack_packet(qp, &ack_pkt, opcode, payload,
 				 res->cur_psn, AETH_ACK_UNLIMITED);
 	if (!skb) {
-		if (mr)
-			rxe_put(mr);
-		return RESPST_ERR_RNR;
+		state = RESPST_ERR_RNR;
+		goto err_out;
 	}
 
 	err = rxe_mr_copy(mr, res->read.va, payload_addr(&ack_pkt),
 			  payload, RXE_FROM_MR_OBJ);
-	if (mr)
-		rxe_put(mr);
 	if (err) {
 		kfree_skb(skb);
-		return RESPST_ERR_RKEY_VIOLATION;
+		state = RESPST_ERR_RKEY_VIOLATION;
+		goto err_out;
 	}
 
 	if (bth_pad(&ack_pkt)) {
@@ -958,9 +957,12 @@  static enum resp_states read_reply(struct rxe_qp *qp,
 		memset(pad, 0, bth_pad(&ack_pkt));
 	}
 
+	/* rxe_xmit_packet always consumes the skb */
 	err = rxe_xmit_packet(qp, &ack_pkt, skb);
-	if (err)
-		return RESPST_ERR_RNR;
+	if (err) {
+		state = RESPST_ERR_RNR;
+		goto err_out;
+	}
 
 	res->read.va += payload;
 	res->read.resid -= payload;
@@ -977,6 +979,9 @@  static enum resp_states read_reply(struct rxe_qp *qp,
 		state = RESPST_CLEANUP;
 	}
 
+err_out:
+	if (mr)
+		rxe_put(mr);
 	return state;
 }