| Message ID | 20210226075515.21371-1-dinghao.liu@zju.edu.cn (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | RDMA/siw: Fix missing check in siw_get_hdr | expand |
-----"Dinghao Liu" <dinghao.liu@zju.edu.cn> wrote: ----- >To: dinghao.liu@zju.edu.cn, kjlu@umn.edu >From: "Dinghao Liu" <dinghao.liu@zju.edu.cn> >Date: 02/26/2021 08:56AM >Cc: "Bernard Metzler" <bmt@zurich.ibm.com>, "Doug Ledford" ><dledford@redhat.com>, "Jason Gunthorpe" <jgg@ziepe.ca>, >linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org >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@zju.edu.cn> >--- > 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 > >
"Bernard Metzler" <BMT@zurich.ibm.com>写道： > -----"Dinghao Liu" <dinghao.liu@zju.edu.cn> wrote: ----- > > >To: dinghao.liu@zju.edu.cn, kjlu@umn.edu > >From: "Dinghao Liu" <dinghao.liu@zju.edu.cn> > >Date: 02/26/2021 08:56AM > >Cc: "Bernard Metzler" <bmt@zurich.ibm.com>, "Doug Ledford" > ><dledford@redhat.com>, "Jason Gunthorpe" <jgg@ziepe.ca>, > >linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org > >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. > It's clear to me, thanks! Regards, Dinghao
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;
We should also check the range of opcode after calling __rdmap_get_opcode() in the else branch to prevent potential overflow. Fixes: 8b6a361b8c482 ("rdma/siw: receive path") Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn> --- drivers/infiniband/sw/siw/siw_qp_rx.c | 10 ++++++++++ 1 file changed, 10 insertions(+)