Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Jia-Ju Bai <baijiaju1990@gmail.com>
To:     zyjzyj2000@gmail.com, jgg@ziepe.ca, leon@kernel.org
Cc:     linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org,
        Jia-Ju Bai <baijiaju1990@gmail.com>,
        TOTE Robot <oslab@tsinghua.edu.cn>
Subject: [PATCH] infiniband: sw: rxe: Add NULL checks for qp->resp.mr
Date:   Fri, 13 Jan 2023 10:35:27 +0800
Message-Id: <20230113023527.728725-1-baijiaju1990@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

In a previous commit 3282a549cf9b, qp->resp.mr could be NULL. Moreover,
in many functions, qp->resp.mr is checked before its dereferences.
However, in some functions, this variable is not checked, and thus NULL 
checks should be added.

These results are reported by a static tool written by myself.

Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
---
 drivers/infiniband/sw/rxe/rxe_resp.c | 47 ++++++++++++++++------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index c74972244f08..2eafa1667a9e 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -621,11 +621,13 @@ static enum resp_states write_data_in(struct rxe_qp *qp,
 	int	err;
 	int data_len = payload_size(pkt);
 
-	err = rxe_mr_copy(qp->resp.mr, qp->resp.va + qp->resp.offset,
-			  payload_addr(pkt), data_len, RXE_TO_MR_OBJ);
-	if (err) {
-		rc = RESPST_ERR_RKEY_VIOLATION;
-		goto out;
+	if (qp->resp.mr) {
+		err = rxe_mr_copy(qp->resp.mr, qp->resp.va + qp->resp.offset,
+				  payload_addr(pkt), data_len, RXE_TO_MR_OBJ);
+		if (err) {
+			rc = RESPST_ERR_RKEY_VIOLATION;
+			goto out;
+		}
 	}
 
 	qp->resp.va += data_len;
@@ -699,11 +701,13 @@ static enum resp_states process_flush(struct rxe_qp *qp,
 		start = res->flush.va;
 		length = res->flush.length;
 	} else { /* level == IB_FLUSH_MR */
-		start = mr->ibmr.iova;
-		length = mr->ibmr.length;
+		if (mr) {
+			start = mr->ibmr.iova;
+			length = mr->ibmr.length;
+		}
 	}
 
-	if (res->flush.type & IB_FLUSH_PERSISTENT) {
+	if (mr && res->flush.type & IB_FLUSH_PERSISTENT) {
 		if (rxe_flush_pmem_iova(mr, start, length))
 			return RESPST_ERR_RKEY_VIOLATION;
 		/* Make data persistent. */
@@ -742,7 +746,7 @@ static enum resp_states atomic_reply(struct rxe_qp *qp,
 		qp->resp.res = res;
 	}
 
-	if (!res->replay) {
+	if (!res->replay && mr) {
 		if (mr->state != RXE_MR_STATE_VALID) {
 			ret = RESPST_ERR_RKEY_VIOLATION;
 			goto out;
@@ -793,15 +797,17 @@ static enum resp_states do_atomic_write(struct rxe_qp *qp,
 	int payload = payload_size(pkt);
 	u64 src, *dst;
 
-	if (mr->state != RXE_MR_STATE_VALID)
+	if (mr && mr->state != RXE_MR_STATE_VALID)
 		return RESPST_ERR_RKEY_VIOLATION;
 
 	memcpy(&src, payload_addr(pkt), payload);
 
-	dst = iova_to_vaddr(mr, qp->resp.va + qp->resp.offset, payload);
-	/* check vaddr is 8 bytes aligned. */
-	if (!dst || (uintptr_t)dst & 7)
-		return RESPST_ERR_MISALIGNED_ATOMIC;
+	if (mr) {
+		dst = iova_to_vaddr(mr, qp->resp.va + qp->resp.offset, payload);
+		/* check vaddr is 8 bytes aligned. */
+		if (!dst || (uintptr_t)dst & 7)
+			return RESPST_ERR_MISALIGNED_ATOMIC;
+	}
 
 	/* Do atomic write after all prior operations have completed */
 	smp_store_release(dst, src);
@@ -1002,13 +1008,14 @@ static enum resp_states read_reply(struct rxe_qp *qp,
 		return RESPST_ERR_RNR;
 	}
 
-	err = rxe_mr_copy(mr, res->read.va, payload_addr(&ack_pkt),
-			  payload, RXE_FROM_MR_OBJ);
-	if (mr)
+	if (mr) {
+		err = rxe_mr_copy(mr, res->read.va, payload_addr(&ack_pkt),
+				  payload, RXE_FROM_MR_OBJ);
 		rxe_put(mr);
-	if (err) {
-		kfree_skb(skb);
-		return RESPST_ERR_RKEY_VIOLATION;
+		if (err) {
+			kfree_skb(skb);
+			return RESPST_ERR_RKEY_VIOLATION;
+		}
 	}
 
 	if (bth_pad(&ack_pkt)) {
-- 
2.34.1