Re: [PATCH] RDMA/siw: Fix missing check in siw_get_hdr

[Bernard Metzler]

-----"Dinghao Liu" <dinghao.liu@xxxxxxxxxx> wrote: -----

>To: dinghao.liu@xxxxxxxxxx, kjlu@xxxxxxx
>From: "Dinghao Liu" <dinghao.liu@xxxxxxxxxx>
>Date: 02/26/2021 08:56AM
>Cc: "Bernard Metzler" <bmt@xxxxxxxxxxxxxx>, "Doug Ledford"
><dledford@xxxxxxxxxx>, "Jason Gunthorpe" <jgg@xxxxxxxx>,
>linux-rdma@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
>Subject: [EXTERNAL] [PATCH] RDMA/siw: Fix missing check in
>siw_get_hdr
>
>We should also check the range of opcode after calling
>__rdmap_get_opcode() in the else branch to prevent potential
>overflow.

Hi Dinghao,
No this is not needed. We always first read the minimum
header information (MPA len, DDP flags, RDMAP opcode,
STag, target offset). Only if we have received that
into local buffer, we check for the opcode this one time.
Now the opcode determines the remaining length of the
variably sized part of the header to be received.

We do not have to check the opcode again, since we
already received and checked it.

Best,
Bernard.

>
>Fixes: 8b6a361b8c482 ("rdma/siw: receive path")
>Signed-off-by: Dinghao Liu <dinghao.liu@xxxxxxxxxx>
>---
> drivers/infiniband/sw/siw/siw_qp_rx.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
>diff --git a/drivers/infiniband/sw/siw/siw_qp_rx.c
>b/drivers/infiniband/sw/siw/siw_qp_rx.c
>index 60116f20653c..301e7fe2c61a 100644
>--- a/drivers/infiniband/sw/siw/siw_qp_rx.c
>+++ b/drivers/infiniband/sw/siw/siw_qp_rx.c
>@@ -1072,6 +1072,16 @@ static int siw_get_hdr(struct siw_rx_stream
>*srx)
> 		siw_dbg_qp(rx_qp(srx), "new header, opcode %u\n", opcode);
> 	} else {
> 		opcode = __rdmap_get_opcode(c_hdr);
>+
>+		if (opcode > RDMAP_TERMINATE) {
>+			pr_warn("siw: received unknown packet type %u\n",
>+				opcode);
>+
>+			siw_init_terminate(rx_qp(srx), TERM_ERROR_LAYER_RDMAP,
>+					   RDMAP_ETYPE_REMOTE_OPERATION,
>+					   RDMAP_ECODE_OPCODE, 0);
>+			return -EINVAL;
>+		}
> 	}
> 	set_rx_fpdu_context(qp, opcode);
> 	frx = qp->rx_fpdu;
>-- 
>2.17.1
>
>