| Message ID | 20221107145057.895747-1-bmt@zurich.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v3] RDMA/siw: Fix immediate work request flush to completion queue. | expand |
On Mon, Nov 07, 2022 at 03:50:57PM +0100, Bernard Metzler wrote: > 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> Please don't add dot at the end of the title. I fixed it locally. Thanks
On Mon, 7 Nov 2022 15:50:57 +0100, Bernard Metzler wrote: > 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. > > [...] Applied, thanks! [1/1] RDMA/siw: Fix immediate work request flush to completion queue. https://git.kernel.org/rdma/rdma/c/bdf1da5df9da68 Best regards,
> -----Original Message----- > From: Leon Romanovsky <leonro@nvidia.com> > Sent: Wednesday, 9 November 2022 14:29 > To: Bernard Metzler <BMT@zurich.ibm.com> > Cc: linux-rdma@vger.kernel.org; jgg@nvidia.com; Olga Kornievskaia > <kolga@netapp.com>; Tom Talpey <tom@talpey.com> > Subject: [EXTERNAL] Re: [PATCH v3] RDMA/siw: Fix immediate work request > flush to completion queue. > > On Mon, Nov 07, 2022 at 03:50:57PM +0100, Bernard Metzler wrote: > > 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> > > Please don't add dot at the end of the title. I fixed it locally. > Thanks for the patience! Best, Bernard.
diff --git a/drivers/infiniband/sw/siw/siw_cq.c b/drivers/infiniband/sw/siw/siw_cq.c index d68e37859e73..acc7bcd538b5 100644 --- 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 3e814cfb298c..906fde1a2a0d 100644 --- 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;
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> --- v2 -> v3: - Fix commit message v1 -> v2: - Change return code of siw_sq_flush_wr() to -EINVAL for unexpected opcodes. --- drivers/infiniband/sw/siw/siw_cq.c | 24 ++++++++++++++-- drivers/infiniband/sw/siw/siw_verbs.c | 40 ++++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 6 deletions(-)