* [PATCH for-next] RDMA/rxe: Fix incorrect responder length checking
@ 2022-12-08 21:09 Bob Pearson
2022-12-09 6:49 ` Daisuke Matsuda (Fujitsu)
2022-12-09 23:30 ` Jason Gunthorpe
0 siblings, 2 replies; 3+ messages in thread
From: Bob Pearson @ 2022-12-08 21:09 UTC (permalink / raw)
To: linux-rdma; +Cc: Bob Pearson
The code in rxe_resp.c at check_length() is incorrect as it
compares pkt->opcode an 8 bit value against various mask bits
which are all higher than 256 so nothing is ever reported.
This patch rewrites this to compare against pkt->mask which is
correct. However this now exposes another error. For UD send
packets the value of the pmtu cannot be determined from qp->mtu.
All that is required here is to later check if the payload fits
into the posted receive buffer in that case.
Fixes: 837a55847ead ("RDMA/rxe: Implement packet length validation on responder")
Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
drivers/infiniband/sw/rxe/rxe_resp.c | 62 ++++++++++++++++------------
1 file changed, 36 insertions(+), 26 deletions(-)
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 6ac544477f3f..42261537772c 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -392,36 +392,46 @@ static enum resp_states check_resource(struct rxe_qp *qp,
return RESPST_CHK_LENGTH;
}
-static enum resp_states check_length(struct rxe_qp *qp,
- struct rxe_pkt_info *pkt)
+static enum resp_states rxe_resp_check_length(struct rxe_qp *qp,
+ struct rxe_pkt_info *pkt)
{
- int mtu = qp->mtu;
- u32 payload = payload_size(pkt);
- u32 dmalen = reth_len(pkt);
-
- /* RoCEv2 packets do not have LRH.
- * Let's skip checking it.
+ /*
+ * See IBA C9-92
+ * For UD QPs we only check if the packet will fit in the
+ * receive buffer later. For rmda operations additional
+ * length checks are performed in check_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->mask & RXE_PAYLOAD_MASK && ((qp_type(qp) == IB_QPT_RC) ||
+ (qp_type(qp) == IB_QPT_UC))) {
+ unsigned int mtu = qp->mtu;
+ unsigned int payload = payload_size(pkt);
+
+ if ((pkt->mask & RXE_START_MASK) &&
+ (pkt->mask & RXE_END_MASK)) {
+ if (unlikely(payload > mtu)) {
+ rxe_dbg_qp(qp, "only packet too long");
+ return RESPST_ERR_LENGTH;
+ }
+ } else if ((pkt->mask & RXE_START_MASK) ||
+ (pkt->mask & RXE_MIDDLE_MASK)) {
+ if (unlikely(payload != mtu)) {
+ rxe_dbg_qp(qp, "first or middle packet not mtu");
+ return RESPST_ERR_LENGTH;
+ }
+ } else if (pkt->mask & RXE_END_MASK) {
+ if (unlikely((payload == 0) || (payload > mtu))) {
+ rxe_dbg_qp(qp, "last packet zero or too long");
+ return RESPST_ERR_LENGTH;
+ }
+ }
}
- if (pkt->opcode & (RXE_WRITE_MASK | RXE_READ_MASK)) {
- if (dmalen > (1 << 31))
+ /* See IBA C9-94 */
+ if (pkt->mask & RXE_RETH_MASK) {
+ if (reth_len(pkt) > (1U << 31)) {
+ rxe_dbg_qp(qp, "dma length too long");
return RESPST_ERR_LENGTH;
+ }
}
return RESPST_CHK_RKEY;
@@ -1397,7 +1407,7 @@ int rxe_responder(void *arg)
state = check_resource(qp, pkt);
break;
case RESPST_CHK_LENGTH:
- state = check_length(qp, pkt);
+ state = rxe_resp_check_length(qp, pkt);
break;
case RESPST_CHK_RKEY:
state = check_rkey(qp, pkt);
--
2.37.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* RE: [PATCH for-next] RDMA/rxe: Fix incorrect responder length checking
2022-12-08 21:09 [PATCH for-next] RDMA/rxe: Fix incorrect responder length checking Bob Pearson
@ 2022-12-09 6:49 ` Daisuke Matsuda (Fujitsu)
2022-12-09 23:30 ` Jason Gunthorpe
1 sibling, 0 replies; 3+ messages in thread
From: Daisuke Matsuda (Fujitsu) @ 2022-12-09 6:49 UTC (permalink / raw)
To: 'Bob Pearson', linux-rdma@vger.kernel.org
On Fri, Dec 9, 2022 6:10 AM Bob Pearson wrote:
> The code in rxe_resp.c at check_length() is incorrect as it
> compares pkt->opcode an 8 bit value against various mask bits
> which are all higher than 256 so nothing is ever reported.
>
> This patch rewrites this to compare against pkt->mask which is
> correct. However this now exposes another error. For UD send
> packets the value of the pmtu cannot be determined from qp->mtu.
> All that is required here is to later check if the payload fits
> into the posted receive buffer in that case.
>
> Fixes: 837a55847ead ("RDMA/rxe: Implement packet length validation on responder")
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
Looks good. Thank you for the fix.
Reviewed-by: Daisuke Matsuda <matsuda-daisuke@fujitsu.com>
> ---
> drivers/infiniband/sw/rxe/rxe_resp.c | 62 ++++++++++++++++------------
> 1 file changed, 36 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index 6ac544477f3f..42261537772c 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -392,36 +392,46 @@ static enum resp_states check_resource(struct rxe_qp *qp,
> return RESPST_CHK_LENGTH;
> }
>
> -static enum resp_states check_length(struct rxe_qp *qp,
> - struct rxe_pkt_info *pkt)
> +static enum resp_states rxe_resp_check_length(struct rxe_qp *qp,
> + struct rxe_pkt_info *pkt)
> {
> - int mtu = qp->mtu;
> - u32 payload = payload_size(pkt);
> - u32 dmalen = reth_len(pkt);
> -
> - /* RoCEv2 packets do not have LRH.
> - * Let's skip checking it.
> + /*
> + * See IBA C9-92
> + * For UD QPs we only check if the packet will fit in the
> + * receive buffer later. For rmda operations additional
> + * length checks are performed in check_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->mask & RXE_PAYLOAD_MASK && ((qp_type(qp) == IB_QPT_RC) ||
> + (qp_type(qp) == IB_QPT_UC))) {
> + unsigned int mtu = qp->mtu;
> + unsigned int payload = payload_size(pkt);
> +
> + if ((pkt->mask & RXE_START_MASK) &&
> + (pkt->mask & RXE_END_MASK)) {
> + if (unlikely(payload > mtu)) {
> + rxe_dbg_qp(qp, "only packet too long");
> + return RESPST_ERR_LENGTH;
> + }
> + } else if ((pkt->mask & RXE_START_MASK) ||
> + (pkt->mask & RXE_MIDDLE_MASK)) {
> + if (unlikely(payload != mtu)) {
> + rxe_dbg_qp(qp, "first or middle packet not mtu");
> + return RESPST_ERR_LENGTH;
> + }
> + } else if (pkt->mask & RXE_END_MASK) {
> + if (unlikely((payload == 0) || (payload > mtu))) {
> + rxe_dbg_qp(qp, "last packet zero or too long");
> + return RESPST_ERR_LENGTH;
> + }
> + }
> }
>
> - if (pkt->opcode & (RXE_WRITE_MASK | RXE_READ_MASK)) {
> - if (dmalen > (1 << 31))
> + /* See IBA C9-94 */
> + if (pkt->mask & RXE_RETH_MASK) {
> + if (reth_len(pkt) > (1U << 31)) {
> + rxe_dbg_qp(qp, "dma length too long");
> return RESPST_ERR_LENGTH;
> + }
> }
>
> return RESPST_CHK_RKEY;
> @@ -1397,7 +1407,7 @@ int rxe_responder(void *arg)
> state = check_resource(qp, pkt);
> break;
> case RESPST_CHK_LENGTH:
> - state = check_length(qp, pkt);
> + state = rxe_resp_check_length(qp, pkt);
> break;
> case RESPST_CHK_RKEY:
> state = check_rkey(qp, pkt);
> --
> 2.37.2
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH for-next] RDMA/rxe: Fix incorrect responder length checking
2022-12-08 21:09 [PATCH for-next] RDMA/rxe: Fix incorrect responder length checking Bob Pearson
2022-12-09 6:49 ` Daisuke Matsuda (Fujitsu)
@ 2022-12-09 23:30 ` Jason Gunthorpe
1 sibling, 0 replies; 3+ messages in thread
From: Jason Gunthorpe @ 2022-12-09 23:30 UTC (permalink / raw)
To: Bob Pearson; +Cc: linux-rdma
On Thu, Dec 08, 2022 at 03:09:46PM -0600, Bob Pearson wrote:
> The code in rxe_resp.c at check_length() is incorrect as it
> compares pkt->opcode an 8 bit value against various mask bits
> which are all higher than 256 so nothing is ever reported.
>
> This patch rewrites this to compare against pkt->mask which is
> correct. However this now exposes another error. For UD send
> packets the value of the pmtu cannot be determined from qp->mtu.
> All that is required here is to later check if the payload fits
> into the posted receive buffer in that case.
>
> Fixes: 837a55847ead ("RDMA/rxe: Implement packet length validation on responder")
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> Reviewed-by: Daisuke Matsuda <matsuda-daisuke@fujitsu.com>
> ---
> drivers/infiniband/sw/rxe/rxe_resp.c | 62 ++++++++++++++++------------
> 1 file changed, 36 insertions(+), 26 deletions(-)
Applied to for next, thanks
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-09 23:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-08 21:09 [PATCH for-next] RDMA/rxe: Fix incorrect responder length checking Bob Pearson
2022-12-09 6:49 ` Daisuke Matsuda (Fujitsu)
2022-12-09 23:30 ` Jason Gunthorpe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.