RDMA/siw: Fix immediate work request flush to completion queue

Correctly set send queue element opcode during immediate work request
flushing in post sendqueue operation, if the QP is in ERROR state.
An undefined ocode value results in out-of-bounds access to an array
for mapping the opcode between siw internal and RDMA core representation
in work completion generation. It resulted in a KASAN BUG report
of type 'global-out-of-bounds' during NFSoRDMA testing.

This patch further fixes a potential case of a malicious user which may
write undefined values for completion queue elements status or opcode,
if the CQ is memory mapped to user land. It avoids the same out-of-bounds
access to arrays for status and opcode mapping as described above.

Fixes: 303ae1cdfdf7 ("rdma/siw: application interface")
Fixes: b0fff7317bb4 ("rdma/siw: completion queue methods")
Reported-by: Olga Kornievskaia <kolga@netapp.com>
Reviewed-by: Tom Talpey <tom@talpey.com>
Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com>
Link: https://lore.kernel.org/r/20221107145057.895747-1-bmt@zurich.ibm.com
Signed-off-by: Leon Romanovsky <leon@kernel.org>

diff --git a/drivers/infiniband/sw/siw/siw_cq.c b/drivers/infiniband/sw/siw/siw_cq.c
index d68e37859e73bf37f0fd64a8a8e120479d93418b..acc7bcd538b534d5c7607b452dc5b25d14cc19ba 100644 (file)
--- a/drivers/infiniband/sw/siw/siw_cq.c
+++ b/drivers/infiniband/sw/siw/siw_cq.c
@@ -56,8 +56,6 @@ int siw_reap_cqe(struct siw_cq *cq, struct ib_wc *wc)
        if (READ_ONCE(cqe->flags) & SIW_WQE_VALID) {
                memset(wc, 0, sizeof(*wc));
                wc->wr_id = cqe->id;
-               wc->status = map_cqe_status[cqe->status].ib;
-               wc->opcode = map_wc_opcode[cqe->opcode];
                wc->byte_len = cqe->bytes;
 
                /*
@@ -71,10 +69,32 @@ int siw_reap_cqe(struct siw_cq *cq, struct ib_wc *wc)
                                wc->wc_flags = IB_WC_WITH_INVALIDATE;
                        }
                        wc->qp = cqe->base_qp;
+                       wc->opcode = map_wc_opcode[cqe->opcode];
+                       wc->status = map_cqe_status[cqe->status].ib;
                        siw_dbg_cq(cq,
                                   "idx %u, type %d, flags %2x, id 0x%pK\n",
                                   cq->cq_get % cq->num_cqe, cqe->opcode,
                                   cqe->flags, (void *)(uintptr_t)cqe->id);
+               } else {
+                       /*
+                        * A malicious user may set invalid opcode or
+                        * status in the user mmapped CQE array.
+                        * Sanity check and correct values in that case
+                        * to avoid out-of-bounds access to global arrays
+                        * for opcode and status mapping.
+                        */
+                       u8 opcode = cqe->opcode;
+                       u16 status = cqe->status;
+
+                       if (opcode >= SIW_NUM_OPCODES) {
+                               opcode = 0;
+                               status = IB_WC_GENERAL_ERR;
+                       } else if (status >= SIW_NUM_WC_STATUS) {
+                               status = IB_WC_GENERAL_ERR;
+                       }
+                       wc->opcode = map_wc_opcode[opcode];
+                       wc->status = map_cqe_status[status].ib;
+
                }
                WRITE_ONCE(cqe->flags, 0);
                cq->cq_get++;

diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
index 3e814cfb298cf8342734fa6917eca138b2ee38b4..906fde1a2a0de2d9ebea9f2e340aacbe2b816fd5 100644 (file)
--- a/drivers/infiniband/sw/siw/siw_verbs.c
+++ b/drivers/infiniband/sw/siw/siw_verbs.c
@@ -676,13 +676,45 @@ static int siw_copy_inline_sgl(const struct ib_send_wr *core_wr,
 static int siw_sq_flush_wr(struct siw_qp *qp, const struct ib_send_wr *wr,
                           const struct ib_send_wr **bad_wr)
 {
-       struct siw_sqe sqe = {};
        int rv = 0;
 
        while (wr) {
-               sqe.id = wr->wr_id;
-               sqe.opcode = wr->opcode;
-               rv = siw_sqe_complete(qp, &sqe, 0, SIW_WC_WR_FLUSH_ERR);
+               struct siw_sqe sqe = {};
+
+               switch (wr->opcode) {
+               case IB_WR_RDMA_WRITE:
+                       sqe.opcode = SIW_OP_WRITE;
+                       break;
+               case IB_WR_RDMA_READ:
+                       sqe.opcode = SIW_OP_READ;
+                       break;
+               case IB_WR_RDMA_READ_WITH_INV:
+                       sqe.opcode = SIW_OP_READ_LOCAL_INV;
+                       break;
+               case IB_WR_SEND:
+                       sqe.opcode = SIW_OP_SEND;
+                       break;
+               case IB_WR_SEND_WITH_IMM:
+                       sqe.opcode = SIW_OP_SEND_WITH_IMM;
+                       break;
+               case IB_WR_SEND_WITH_INV:
+                       sqe.opcode = SIW_OP_SEND_REMOTE_INV;
+                       break;
+               case IB_WR_LOCAL_INV:
+                       sqe.opcode = SIW_OP_INVAL_STAG;
+                       break;
+               case IB_WR_REG_MR:
+                       sqe.opcode = SIW_OP_REG_MR;
+                       break;
+               default:
+                       rv = -EINVAL;
+                       break;
+               }
+               if (!rv) {
+                       sqe.id = wr->wr_id;
+                       rv = siw_sqe_complete(qp, &sqe, 0,
+                                             SIW_WC_WR_FLUSH_ERR);
+               }
                if (rv) {
                        if (bad_wr)
                                *bad_wr = wr;