All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] RDMA/rxe: Implement packet length validation on responder
@ 2022-11-07  5:53 Daisuke Matsuda
  2022-11-07  6:16 ` Zhu Yanjun
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Daisuke Matsuda @ 2022-11-07  5:53 UTC (permalink / raw)
  To: leonro, jgg, zyjzyj2000, linux-rdma; +Cc: lizhijian, Daisuke Matsuda

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(-)

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] RDMA/rxe: Implement packet length validation on responder
  2022-11-07  5:53 [PATCH v2] RDMA/rxe: Implement packet length validation on responder Daisuke Matsuda
@ 2022-11-07  6:16 ` Zhu Yanjun
  2022-11-08  9:00 ` lizhijian
  2022-11-09 17:57 ` Leon Romanovsky
  2 siblings, 0 replies; 4+ messages in thread
From: Zhu Yanjun @ 2022-11-07  6:16 UTC (permalink / raw)
  To: Daisuke Matsuda; +Cc: leonro, jgg, linux-rdma, lizhijian

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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] RDMA/rxe: Implement packet length validation on responder
  2022-11-07  5:53 [PATCH v2] RDMA/rxe: Implement packet length validation on responder Daisuke Matsuda
  2022-11-07  6:16 ` Zhu Yanjun
@ 2022-11-08  9:00 ` lizhijian
  2022-11-09 17:57 ` Leon Romanovsky
  2 siblings, 0 replies; 4+ messages in thread
From: lizhijian @ 2022-11-08  9:00 UTC (permalink / raw)
  To: Daisuke Matsuda (Fujitsu), leonro, jgg, zyjzyj2000, linux-rdma



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,

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] RDMA/rxe: Implement packet length validation on responder
  2022-11-07  5:53 [PATCH v2] RDMA/rxe: Implement packet length validation on responder Daisuke Matsuda
  2022-11-07  6:16 ` Zhu Yanjun
  2022-11-08  9:00 ` lizhijian
@ 2022-11-09 17:57 ` Leon Romanovsky
  2 siblings, 0 replies; 4+ messages in thread
From: Leon Romanovsky @ 2022-11-09 17:57 UTC (permalink / raw)
  To: linux-rdma, zyjzyj2000, Jason Gunthorpe, Leon Romanovsky,
	Daisuke Matsuda
  Cc: lizhijian

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,
-- 
Leon Romanovsky <leon@kernel.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-11-09 18:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-07  5:53 [PATCH v2] RDMA/rxe: Implement packet length validation on responder Daisuke Matsuda
2022-11-07  6:16 ` Zhu Yanjun
2022-11-08  9:00 ` lizhijian
2022-11-09 17:57 ` Leon Romanovsky

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.