在 2023/1/13 10:35, Jia-Ju Bai 写道:
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.
IMO， we should analyze the code snippet one by one.
And it is not good to add "NULL check" without futher investigations.
Zhu Yanjun
These results are reported by a static tool written by myself.
Signed-off-by: Jia-Ju Bai <baijiaju1990@xxxxxxxxx>
Reported-by: TOTE Robot <oslab@xxxxxxxxxxxxxxx>
---
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)) {
