| Message ID | 20221107055338.357184-1-matsuda-daisuke@fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] RDMA/rxe: Implement packet length validation on responder | expand |
On Mon, Nov 7, 2022 at 1:54 PM Daisuke Matsuda <matsuda-daisuke@fujitsu.com> wrote: > > The function check_length() is supposed to check the length of inbound > packets on responder, but it actually has been a stub since the driver was > born. Let it check the payload length and the DMA length. > > Signed-off-by: Daisuke Matsuda <matsuda-daisuke@fujitsu.com> > --- > FOR REVIEWERS > I referred to IB Specification Vol 1-Revision-1.5 to create this patch. > Please see 9.7.4.1.6 (page.330). > > v2: Fixed the conditional for 'last' packets. Thanks, Zhijian. It had better send the v2 patch in a new mail thread. Acked-by: Zhu Yanjun <zyjzyj2000@gmail.com> Zhu Yanjun > > drivers/infiniband/sw/rxe/rxe_resp.c | 34 ++++++++++++++++++++++------ > 1 file changed, 27 insertions(+), 7 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c > index c32bc12cc82f..382d2053db43 100644 > --- a/drivers/infiniband/sw/rxe/rxe_resp.c > +++ b/drivers/infiniband/sw/rxe/rxe_resp.c > @@ -393,16 +393,36 @@ static enum resp_states check_resource(struct rxe_qp *qp, > static enum resp_states check_length(struct rxe_qp *qp, > struct rxe_pkt_info *pkt) > { > - switch (qp_type(qp)) { > - case IB_QPT_RC: > - return RESPST_CHK_RKEY; > + int mtu = qp->mtu; > + u32 payload = payload_size(pkt); > + u32 dmalen = reth_len(pkt); > > - case IB_QPT_UC: > - return RESPST_CHK_RKEY; > + /* RoCEv2 packets do not have LRH. > + * Let's skip checking it. > + */ > > - default: > - return RESPST_CHK_RKEY; > + if ((pkt->opcode & RXE_START_MASK) && > + (pkt->opcode & RXE_END_MASK)) { > + /* "only" packets */ > + if (payload > mtu) > + return RESPST_ERR_LENGTH; > + } else if ((pkt->opcode & RXE_START_MASK) || > + (pkt->opcode & RXE_MIDDLE_MASK)) { > + /* "first" or "middle" packets */ > + if (payload != mtu) > + return RESPST_ERR_LENGTH; > + } else if (pkt->opcode & RXE_END_MASK) { > + /* "last" packets */ > + if ((payload == 0) || (payload > mtu)) > + return RESPST_ERR_LENGTH; > + } > + > + if (pkt->opcode & (RXE_WRITE_MASK | RXE_READ_MASK)) { > + if (dmalen > (1 << 31)) > + return RESPST_ERR_LENGTH; > } > + > + return RESPST_CHK_RKEY; > } > > static enum resp_states check_rkey(struct rxe_qp *qp, > -- > 2.31.1 >
On 07/11/2022 13:53, Daisuke Matsuda wrote: > The function check_length() is supposed to check the length of inbound > packets on responder, but it actually has been a stub since the driver was > born. Let it check the payload length and the DMA length. > > Signed-off-by: Daisuke Matsuda <matsuda-daisuke@fujitsu.com> Looks good to me Reviewed-by: Li Zhijian <lizhijian@fujitsu.com> > --- > FOR REVIEWERS > I referred to IB Specification Vol 1-Revision-1.5 to create this patch. > Please see 9.7.4.1.6 (page.330). > > v2: Fixed the conditional for 'last' packets. Thanks, Zhijian. > > drivers/infiniband/sw/rxe/rxe_resp.c | 34 ++++++++++++++++++++++------ > 1 file changed, 27 insertions(+), 7 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c > index c32bc12cc82f..382d2053db43 100644 > --- a/drivers/infiniband/sw/rxe/rxe_resp.c > +++ b/drivers/infiniband/sw/rxe/rxe_resp.c > @@ -393,16 +393,36 @@ static enum resp_states check_resource(struct rxe_qp *qp, > static enum resp_states check_length(struct rxe_qp *qp, > struct rxe_pkt_info *pkt) > { > - switch (qp_type(qp)) { > - case IB_QPT_RC: > - return RESPST_CHK_RKEY; > + int mtu = qp->mtu; > + u32 payload = payload_size(pkt); > + u32 dmalen = reth_len(pkt); > > - case IB_QPT_UC: > - return RESPST_CHK_RKEY; > + /* RoCEv2 packets do not have LRH. > + * Let's skip checking it. > + */ > > - default: > - return RESPST_CHK_RKEY; > + if ((pkt->opcode & RXE_START_MASK) && > + (pkt->opcode & RXE_END_MASK)) { > + /* "only" packets */ > + if (payload > mtu) > + return RESPST_ERR_LENGTH; > + } else if ((pkt->opcode & RXE_START_MASK) || > + (pkt->opcode & RXE_MIDDLE_MASK)) { > + /* "first" or "middle" packets */ > + if (payload != mtu) > + return RESPST_ERR_LENGTH; > + } else if (pkt->opcode & RXE_END_MASK) { > + /* "last" packets */ > + if ((payload == 0) || (payload > mtu)) > + return RESPST_ERR_LENGTH; > + } > + > + if (pkt->opcode & (RXE_WRITE_MASK | RXE_READ_MASK)) { > + if (dmalen > (1 << 31)) > + return RESPST_ERR_LENGTH; > } > + > + return RESPST_CHK_RKEY; > } > > static enum resp_states check_rkey(struct rxe_qp *qp,
On Mon, 7 Nov 2022 14:53:38 +0900, Daisuke Matsuda wrote: > The function check_length() is supposed to check the length of inbound > packets on responder, but it actually has been a stub since the driver was > born. Let it check the payload length and the DMA length. > > Applied, thanks! [1/1] RDMA/rxe: Implement packet length validation on responder https://git.kernel.org/rdma/rdma/c/837a55847ead27 Best regards,
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c index c32bc12cc82f..382d2053db43 100644 --- a/drivers/infiniband/sw/rxe/rxe_resp.c +++ b/drivers/infiniband/sw/rxe/rxe_resp.c @@ -393,16 +393,36 @@ static enum resp_states check_resource(struct rxe_qp *qp, static enum resp_states check_length(struct rxe_qp *qp, struct rxe_pkt_info *pkt) { - switch (qp_type(qp)) { - case IB_QPT_RC: - return RESPST_CHK_RKEY; + int mtu = qp->mtu; + u32 payload = payload_size(pkt); + u32 dmalen = reth_len(pkt); - case IB_QPT_UC: - return RESPST_CHK_RKEY; + /* RoCEv2 packets do not have LRH. + * Let's skip checking it. + */ - default: - return RESPST_CHK_RKEY; + if ((pkt->opcode & RXE_START_MASK) && + (pkt->opcode & RXE_END_MASK)) { + /* "only" packets */ + if (payload > mtu) + return RESPST_ERR_LENGTH; + } else if ((pkt->opcode & RXE_START_MASK) || + (pkt->opcode & RXE_MIDDLE_MASK)) { + /* "first" or "middle" packets */ + if (payload != mtu) + return RESPST_ERR_LENGTH; + } else if (pkt->opcode & RXE_END_MASK) { + /* "last" packets */ + if ((payload == 0) || (payload > mtu)) + return RESPST_ERR_LENGTH; + } + + if (pkt->opcode & (RXE_WRITE_MASK | RXE_READ_MASK)) { + if (dmalen > (1 << 31)) + return RESPST_ERR_LENGTH; } + + return RESPST_CHK_RKEY; } static enum resp_states check_rkey(struct rxe_qp *qp,
The function check_length() is supposed to check the length of inbound packets on responder, but it actually has been a stub since the driver was born. Let it check the payload length and the DMA length. Signed-off-by: Daisuke Matsuda <matsuda-daisuke@fujitsu.com> --- FOR REVIEWERS I referred to IB Specification Vol 1-Revision-1.5 to create this patch. Please see 9.7.4.1.6 (page.330). v2: Fixed the conditional for 'last' packets. Thanks, Zhijian. drivers/infiniband/sw/rxe/rxe_resp.c | 34 ++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 7 deletions(-)