> -----Original Message-----
> From: Dan Carpenter <error27@xxxxxxxxx>
> Sent: Tuesday, 15 November 2022 14:45
> To: Bernard Metzler <BMT@xxxxxxxxxxxxxx>
> Cc: linux-rdma@xxxxxxxxxxxxxxx
> Subject: [EXTERNAL] [bug report] RDMA/siw: Fix immediate work request flush
> to completion queue
>
> Hello Bernard Metzler,
>
> The patch bdf1da5df9da: "RDMA/siw: Fix immediate work request flush
> to completion queue" from Nov 7, 2022, leads to the following Smatch
> static checker warning:
Hi Dan Carpenter,
that’s a fantastic tool! I shall make use of it
to catch those bad typos.
It should have been SIW_WC_GENERAL_ERR
instead of IB_WC_GENERAL_ERR
I'll send a fix asap.
Thanks very much!
Bernard.
>
> drivers/infiniband/sw/siw/siw_cq.c:96 siw_reap_cqe()
> error: buffer overflow 'map_cqe_status' 10 <= 21
>
> drivers/infiniband/sw/siw/siw_cq.c
> 48 int siw_reap_cqe(struct siw_cq *cq, struct ib_wc *wc)
> 49 {
> 50 struct siw_cqe *cqe;
> 51 unsigned long flags;
> 52
> 53 spin_lock_irqsave(&cq->lock, flags);
> 54
> 55 cqe = &cq->queue[cq->cq_get % cq->num_cqe];
> 56 if (READ_ONCE(cqe->flags) & SIW_WQE_VALID) {
> 57 memset(wc, 0, sizeof(*wc));
> 58 wc->wr_id = cqe->id;
> 59 wc->byte_len = cqe->bytes;
> 60
> 61 /*
> 62 * During CQ flush, also user land CQE's may get
> 63 * reaped here, which do not hold a QP reference
> 64 * and do not qualify for memory extension verbs.
> 65 */
> 66 if (likely(rdma_is_kernel_res(&cq->base_cq.res))) {
> 67 if (cqe->flags & SIW_WQE_REM_INVAL) {
> 68 wc->ex.invalidate_rkey = cqe-
> >inval_stag;
> 69 wc->wc_flags =
> IB_WC_WITH_INVALIDATE;
> 70 }
> 71 wc->qp = cqe->base_qp;
> 72 wc->opcode = map_wc_opcode[cqe->opcode];
> 73 wc->status = map_cqe_status[cqe->status].ib;
> 74 siw_dbg_cq(cq,
> 75 "idx %u, type %d, flags %2x, id
> 0x%pK\n",
> 76 cq->cq_get % cq->num_cqe, cqe-
> >opcode,
> 77 cqe->flags, (void
> *)(uintptr_t)cqe->id);
> 78 } else {
> 79 /*
> 80 * A malicious user may set invalid opcode
> or
> 81 * status in the user mmapped CQE array.
> 82 * Sanity check and correct values in that
> case
> 83 * to avoid out-of-bounds access to global
> arrays
> 84 * for opcode and status mapping.
> 85 */
> 86 u8 opcode = cqe->opcode;
> 87 u16 status = cqe->status;
> 88
> 89 if (opcode >= SIW_NUM_OPCODES) {
> 90 opcode = 0;
> 91 status = IB_WC_GENERAL_ERR;
>
> IB_WC_GENERAL_ERR is 21.
>
> 92 } else if (status >= SIW_NUM_WC_STATUS) {
> 93 status = IB_WC_GENERAL_ERR;
>
> Here too.
>
> 94 }
> 95 wc->opcode = map_wc_opcode[opcode];
> --> 96 wc->status = map_cqe_status[status].ib;
> ^^^^^^^^^^^^^^^
> Out of bounds.
>
> 97
> 98 }
> 99 WRITE_ONCE(cqe->flags, 0);
> 100 cq->cq_get++;
> 101
> 102 spin_unlock_irqrestore(&cq->lock, flags);
> 103
> 104 return 1;
> 105 }
> 106 spin_unlock_irqrestore(&cq->lock, flags);
> 107
> 108 return 0;
> 109 }
>
> regards,
> dan carpenter
