| Message ID | 20221028090438.2685345-1-matsuda-daisuke@fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RDMA/rxe: Implement packet length validation on responder | expand |
On 28/10/2022 17:04, 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> > --- > 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). > > drivers/infiniband/sw/rxe/rxe_resp.c | 36 ++++++++++++++++++++++------ > 1 file changed, 29 insertions(+), 7 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c > index ed5a09e86417..62e3a8195072 100644 > --- a/drivers/infiniband/sw/rxe/rxe_resp.c > +++ b/drivers/infiniband/sw/rxe/rxe_resp.c > @@ -390,16 +390,38 @@ 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 ((pkt->paylen == 0) || (pkt->paylen > mtu)) As per IBA spec, i didn't see any difference about the 'packet payload length' from others, so May I know why here you are using 'pkt->paylen' instead of payload ? Thanks Zhijian > + 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 Sat, Nov 5, 2022 6:37 PM Li, Zhijian wrote: > On 28/10/2022 17:04, 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> > > --- > > 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). > > > > drivers/infiniband/sw/rxe/rxe_resp.c | 36 ++++++++++++++++++++++------ > > 1 file changed, 29 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c > > index ed5a09e86417..62e3a8195072 100644 > > --- a/drivers/infiniband/sw/rxe/rxe_resp.c > > +++ b/drivers/infiniband/sw/rxe/rxe_resp.c > > @@ -390,16 +390,38 @@ 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 ((pkt->paylen == 0) || (pkt->paylen > mtu)) > > As per IBA spec, i didn't see any difference about the 'packet payload > length' from others, > > so May I know why here you are using 'pkt->paylen' instead of payload ? Thank you for taking a look. It seems this is my mistake. I forgot to replace them with 'payload'. I'll post v2 later. Actually 'pkt->paylen' is larger than 'payload': pkt->paylen = (IB BTH) + (IB payload) + (ICRC) Daisuke > > > Thanks > > Zhijian > > > > > + 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,
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c index ed5a09e86417..62e3a8195072 100644 --- a/drivers/infiniband/sw/rxe/rxe_resp.c +++ b/drivers/infiniband/sw/rxe/rxe_resp.c @@ -390,16 +390,38 @@ 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 ((pkt->paylen == 0) || (pkt->paylen > 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). drivers/infiniband/sw/rxe/rxe_resp.c | 36 ++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 7 deletions(-)