mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] RDMA/rxe: Fix out-of-range unsigned-to-signed conversion for RDMA message in 2GiB size
@ 2026-09-15  6:45 Honggang LI
  2026-09-15 11:49 ` Leon Romanovsky
  0 siblings, 1 reply; 4+ messages in thread
From: Honggang LI @ 2026-09-15  6:45 UTC (permalink / raw)
  To: zyjzyj2000, jgg, leon; +Cc: linux-rdma, linux-kernel, Honggang LI

When RDMA READ request for 2GiB in single WR, res->read.resid is u32
0x80000000, which is INT_MIN (-2147483648).

    payload = min_t(int, res->read.resid, mtu);

The `min_t` function will return -2147483648 for payload. The wrong
size is propagated through call chain, `read_reply` -> `prepare_ack_packet`
-> `rxe_init_packet` -> `alloc_skb` . `alloc_skb` failed because of
invalid size. The passive side failed to emit response packet for RDMA
READ request.

After fixed it, the active side of RDMA READ failed with error code
"IB_WC_LOC_PROT_ERR". When RDMA_READ_RESPONSE_FIRST packet recived by
the active side, `do_read` call `copy_data`. dma->resid is u32 0x80000000.

   int resid	= dma->resid;

This conversion set resid to -2147483648. `copy_data` abort as length
greater than resid. Change resid to int64_t fixes this issue.

RDMA SEND and WRITE 2GiB message works too, after fixed these two bugs.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Honggang LI <honggangli@163.com>
---
 drivers/infiniband/sw/rxe/rxe_loc.h  | 2 +-
 drivers/infiniband/sw/rxe/rxe_mr.c   | 4 ++--
 drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 64d636bf80fd..9acf2494bb94 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -64,7 +64,7 @@ int rxe_flush_pmem_iova(struct rxe_mr *mr, u64 iova, unsigned int length);
 int rxe_mr_copy(struct rxe_mr *mr, u64 iova, void *addr,
 		unsigned int length, enum rxe_mr_copy_dir dir);
 int copy_data(struct rxe_pd *pd, int access, struct rxe_dma_info *dma,
-	      void *addr, int length, enum rxe_mr_copy_dir dir);
+	      void *addr, int64_t length, enum rxe_mr_copy_dir dir);
 int rxe_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg,
 		  int sg_nents, unsigned int *sg_offset);
 enum resp_states rxe_mr_do_atomic_op(struct rxe_mr *mr, u64 iova, int opcode,
diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index 71d9ea477289..1a9005f11079 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -418,13 +418,13 @@ int copy_data(
 	int			access,
 	struct rxe_dma_info	*dma,
 	void			*addr,
-	int			length,
+	int64_t			length,
 	enum rxe_mr_copy_dir	dir)
 {
 	int			bytes;
 	struct rxe_sge		*sge	= &dma->sge[dma->cur_sge];
 	int			offset	= dma->sge_offset;
-	int			resid	= dma->resid;
+	int64_t			resid	= dma->resid;
 	struct rxe_mr		*mr	= NULL;
 	u64			iova;
 	int			err;
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 02b16e2b49b8..52bce17511dc 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -982,7 +982,7 @@ static enum resp_states read_reply(struct rxe_qp *qp,
 
 	res->state = rdatm_res_state_next;
 
-	payload = min_t(int, res->read.resid, mtu);
+	payload = min_t(u32, res->read.resid, mtu);
 
 	skb = prepare_ack_packet(qp, &ack_pkt, opcode, payload,
 				 res->cur_psn, AETH_ACK_UNLIMITED);
-- 
2.54.0


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

* Re: [PATCH] RDMA/rxe: Fix out-of-range unsigned-to-signed conversion for RDMA message in 2GiB size
  2026-09-15  6:45 [PATCH] RDMA/rxe: Fix out-of-range unsigned-to-signed conversion for RDMA message in 2GiB size Honggang LI
@ 2026-09-15 11:49 ` Leon Romanovsky
  2026-09-16  3:23   ` Honggang LI
  0 siblings, 1 reply; 4+ messages in thread
From: Leon Romanovsky @ 2026-09-15 11:49 UTC (permalink / raw)
  To: Honggang LI; +Cc: zyjzyj2000, jgg, linux-rdma, linux-kernel

On Tue, Sep 15, 2026 at 02:45:32PM +0800, Honggang LI wrote:
> When RDMA READ request for 2GiB in single WR, res->read.resid is u32
> 0x80000000, which is INT_MIN (-2147483648).
> 
>     payload = min_t(int, res->read.resid, mtu);
> 
> The `min_t` function will return -2147483648 for payload. The wrong
> size is propagated through call chain, `read_reply` -> `prepare_ack_packet`
> -> `rxe_init_packet` -> `alloc_skb` . `alloc_skb` failed because of
> invalid size. The passive side failed to emit response packet for RDMA
> READ request.
> 
> After fixed it, the active side of RDMA READ failed with error code
> "IB_WC_LOC_PROT_ERR". When RDMA_READ_RESPONSE_FIRST packet recived by
> the active side, `do_read` call `copy_data`. dma->resid is u32 0x80000000.
> 
>    int resid	= dma->resid;
> 
> This conversion set resid to -2147483648. `copy_data` abort as length
> greater than resid. Change resid to int64_t fixes this issue.

Why not size_t?

> 
> RDMA SEND and WRITE 2GiB message works too, after fixed these two bugs.
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Honggang LI <honggangli@163.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_loc.h  | 2 +-
>  drivers/infiniband/sw/rxe/rxe_mr.c   | 4 ++--
>  drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
> index 64d636bf80fd..9acf2494bb94 100644
> --- a/drivers/infiniband/sw/rxe/rxe_loc.h
> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
> @@ -64,7 +64,7 @@ int rxe_flush_pmem_iova(struct rxe_mr *mr, u64 iova, unsigned int length);
>  int rxe_mr_copy(struct rxe_mr *mr, u64 iova, void *addr,
>  		unsigned int length, enum rxe_mr_copy_dir dir);
>  int copy_data(struct rxe_pd *pd, int access, struct rxe_dma_info *dma,
> -	      void *addr, int length, enum rxe_mr_copy_dir dir);
> +	      void *addr, int64_t length, enum rxe_mr_copy_dir dir);
>  int rxe_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg,
>  		  int sg_nents, unsigned int *sg_offset);
>  enum resp_states rxe_mr_do_atomic_op(struct rxe_mr *mr, u64 iova, int opcode,
> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> index 71d9ea477289..1a9005f11079 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> @@ -418,13 +418,13 @@ int copy_data(
>  	int			access,
>  	struct rxe_dma_info	*dma,
>  	void			*addr,
> -	int			length,
> +	int64_t			length,
>  	enum rxe_mr_copy_dir	dir)
>  {
>  	int			bytes;
>  	struct rxe_sge		*sge	= &dma->sge[dma->cur_sge];
>  	int			offset	= dma->sge_offset;
> -	int			resid	= dma->resid;
> +	int64_t			resid	= dma->resid;
>  	struct rxe_mr		*mr	= NULL;
>  	u64			iova;
>  	int			err;
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index 02b16e2b49b8..52bce17511dc 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -982,7 +982,7 @@ static enum resp_states read_reply(struct rxe_qp *qp,
>  
>  	res->state = rdatm_res_state_next;
>  
> -	payload = min_t(int, res->read.resid, mtu);
> +	payload = min_t(u32, res->read.resid, mtu);

Why don't we use the proper types from the start to avoid the need for
u32 casts?

Thanks

>  
>  	skb = prepare_ack_packet(qp, &ack_pkt, opcode, payload,
>  				 res->cur_psn, AETH_ACK_UNLIMITED);
> -- 
> 2.54.0
> 

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

* Re: [PATCH] RDMA/rxe: Fix out-of-range unsigned-to-signed conversion for RDMA message in 2GiB size
  2026-09-15 11:49 ` Leon Romanovsky
@ 2026-09-16  3:23   ` Honggang LI
  2026-09-16  5:43     ` Leon Romanovsky
  0 siblings, 1 reply; 4+ messages in thread
From: Honggang LI @ 2026-09-16  3:23 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: zyjzyj2000, jgg, linux-rdma, linux-kernel

On Tue, Sep 15, 2026 at 02:49:14PM +0300, Leon Romanovsky wrote:
> > After fixed it, the active side of RDMA READ failed with error code
> > "IB_WC_LOC_PROT_ERR". When RDMA_READ_RESPONSE_FIRST packet recived by
> > the active side, `do_read` call `copy_data`. dma->resid is u32 0x80000000.
> > 
> >    int resid	= dma->resid;
> > 
> > This conversion set resid to -2147483648. `copy_data` abort as length
> > greater than resid. Change resid to int64_t fixes this issue.
> 
> Why not size_t?

First, size_t is u64. dma->resid is u32. If use unsigned type, u32 is enough.

Second, I'm not sure it is right to use unsigned type. In `copy_data`,
the loop terminate on negative value of `length`.

Use int64_t is safe and minimal changes of the code.

int copy_data(
..................
	while (length > 0) {
	^^^^^^^^^^^^^^^^^^^^^^^^
		bytes = length;
..............
		if (bytes > sge->length - offset)
			bytes = sge->length - offset;

		if (bytes > 0) {
			iova = sge->addr + offset;
			err = rxe_mr_copy(mr, iova, addr, bytes, dir);

			offset	+= bytes;
			resid	-= bytes;
			length	-= bytes;
			addr	+= bytes;
		}
	}

> 
> >  
> > -	payload = min_t(int, res->read.resid, mtu);
> > +	payload = min_t(u32, res->read.resid, mtu);
> 
> Why don't we use the proper types from the start to avoid the need for
> u32 casts?

Again, minimal the changes with u32 casts. We need something like this
to use u32.

---
 drivers/infiniband/sw/rxe/rxe_loc.h  |  2 +-
 drivers/infiniband/sw/rxe/rxe_net.c  |  2 +-
 drivers/infiniband/sw/rxe/rxe_resp.c | 15 ++++++++-------
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 64d636bf80fd..ceb9321add96 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -91,7 +91,7 @@ void rxe_mw_cleanup(struct rxe_pool_elem *elem);
 
 /* rxe_net.c */
 struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
-				int paylen, struct rxe_pkt_info *pkt);
+				u32 paylen, struct rxe_pkt_info *pkt);
 int rxe_prepare(struct rxe_av *av, struct rxe_pkt_info *pkt,
 		struct sk_buff *skb);
 int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 53daaf4c1eb2..f548f312b393 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -536,7 +536,7 @@ int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
 }
 
 struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
-				int paylen, struct rxe_pkt_info *pkt)
+				u32 paylen, struct rxe_pkt_info *pkt)
 {
 	unsigned int hdr_len;
 	struct sk_buff *skb = NULL;
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 02b16e2b49b8..383706de0b29 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -827,20 +827,21 @@ static enum resp_states atomic_write_reply(struct rxe_qp *qp,
 static struct sk_buff *prepare_ack_packet(struct rxe_qp *qp,
 					  struct rxe_pkt_info *ack,
 					  int opcode,
-					  int payload,
+					  u32 payload,
 					  u32 psn,
 					  u8 syndrome)
 {
 	struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
 	struct sk_buff *skb;
-	int paylen;
-	int pad;
+	u32 paylen;
+	u32 pad = 0;
 	int err;
 
 	/*
 	 * allocate packet
 	 */
-	pad = (-payload) & 0x3;
+	if (payload % 4)
+		pad = 4 - payload % 4;
 	paylen = rxe_opcode[opcode].length + payload + pad + RXE_ICRC_SIZE;
 
 	skb = rxe_init_packet(rxe, &qp->pri_av, paylen, ack);
@@ -934,9 +935,9 @@ static enum resp_states read_reply(struct rxe_qp *qp,
 {
 	struct rxe_pkt_info ack_pkt;
 	struct sk_buff *skb;
-	int mtu = qp->mtu;
+	u32 mtu = qp->mtu;
 	enum resp_states state;
-	int payload;
+	u32 payload;
 	int opcode;
 	int err;
 	struct resp_res *res = qp->resp.res;
@@ -982,7 +983,7 @@ static enum resp_states read_reply(struct rxe_qp *qp,
 
 	res->state = rdatm_res_state_next;
 
-	payload = min_t(int, res->read.resid, mtu);
+	payload = min(res->read.resid, mtu);
 
 	skb = prepare_ack_packet(qp, &ack_pkt, opcode, payload,
 				 res->cur_psn, AETH_ACK_UNLIMITED);

---
Thanks				 


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

* Re: [PATCH] RDMA/rxe: Fix out-of-range unsigned-to-signed conversion for RDMA message in 2GiB size
  2026-09-16  3:23   ` Honggang LI
@ 2026-09-16  5:43     ` Leon Romanovsky
  0 siblings, 0 replies; 4+ messages in thread
From: Leon Romanovsky @ 2026-09-16  5:43 UTC (permalink / raw)
  To: Honggang LI; +Cc: zyjzyj2000, jgg, linux-rdma, linux-kernel

On Wed, Sep 16, 2026 at 11:23:37AM +0800, Honggang LI wrote:
> On Tue, Sep 15, 2026 at 02:49:14PM +0300, Leon Romanovsky wrote:
> > > After fixed it, the active side of RDMA READ failed with error code
> > > "IB_WC_LOC_PROT_ERR". When RDMA_READ_RESPONSE_FIRST packet recived by
> > > the active side, `do_read` call `copy_data`. dma->resid is u32 0x80000000.
> > > 
> > >    int resid	= dma->resid;
> > > 
> > > This conversion set resid to -2147483648. `copy_data` abort as length
> > > greater than resid. Change resid to int64_t fixes this issue.
> > 
> > Why not size_t?
> 
> First, size_t is u64. dma->resid is u32. If use unsigned type, u32 is enough.

"Change resid to int64_t": originally, you used u64, which is equivalent
to size_t on 64-bit systems.

> 
> Second, I'm not sure it is right to use unsigned type. In `copy_data`,
> the loop terminate on negative value of `length`.

Length should never be negative. This is another example of rather
unclean code.

> 
> Use int64_t is safe and minimal changes of the code.
> 
> int copy_data(
> ..................
> 	while (length > 0) {
> 	^^^^^^^^^^^^^^^^^^^^^^^^
> 		bytes = length;
> ..............
> 		if (bytes > sge->length - offset)
> 			bytes = sge->length - offset;
> 
> 		if (bytes > 0) {
> 			iova = sge->addr + offset;
> 			err = rxe_mr_copy(mr, iova, addr, bytes, dir);
> 
> 			offset	+= bytes;
> 			resid	-= bytes;
> 			length	-= bytes;
> 			addr	+= bytes;
> 		}
> 	}
> 
> > 
> > >  
> > > -	payload = min_t(int, res->read.resid, mtu);
> > > +	payload = min_t(u32, res->read.resid, mtu);
> > 
> > Why don't we use the proper types from the start to avoid the need for
> > u32 casts?
> 
> Again, minimal the changes with u32 casts. We need something like this
> to use u32.

1. `int64_t` is not commonly used in the kernel. Please use `s64` instead.
2. I'm looking for a correct change and don't care whether it is minimal.

Thanks

> 
> ---
>  drivers/infiniband/sw/rxe/rxe_loc.h  |  2 +-
>  drivers/infiniband/sw/rxe/rxe_net.c  |  2 +-
>  drivers/infiniband/sw/rxe/rxe_resp.c | 15 ++++++++-------
>  3 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
> index 64d636bf80fd..ceb9321add96 100644
> --- a/drivers/infiniband/sw/rxe/rxe_loc.h
> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
> @@ -91,7 +91,7 @@ void rxe_mw_cleanup(struct rxe_pool_elem *elem);
>  
>  /* rxe_net.c */
>  struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
> -				int paylen, struct rxe_pkt_info *pkt);
> +				u32 paylen, struct rxe_pkt_info *pkt);
>  int rxe_prepare(struct rxe_av *av, struct rxe_pkt_info *pkt,
>  		struct sk_buff *skb);
>  int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 53daaf4c1eb2..f548f312b393 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -536,7 +536,7 @@ int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
>  }
>  
>  struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
> -				int paylen, struct rxe_pkt_info *pkt)
> +				u32 paylen, struct rxe_pkt_info *pkt)
>  {
>  	unsigned int hdr_len;
>  	struct sk_buff *skb = NULL;
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index 02b16e2b49b8..383706de0b29 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -827,20 +827,21 @@ static enum resp_states atomic_write_reply(struct rxe_qp *qp,
>  static struct sk_buff *prepare_ack_packet(struct rxe_qp *qp,
>  					  struct rxe_pkt_info *ack,
>  					  int opcode,
> -					  int payload,
> +					  u32 payload,
>  					  u32 psn,
>  					  u8 syndrome)
>  {
>  	struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
>  	struct sk_buff *skb;
> -	int paylen;
> -	int pad;
> +	u32 paylen;
> +	u32 pad = 0;
>  	int err;
>  
>  	/*
>  	 * allocate packet
>  	 */
> -	pad = (-payload) & 0x3;
> +	if (payload % 4)
> +		pad = 4 - payload % 4;
>  	paylen = rxe_opcode[opcode].length + payload + pad + RXE_ICRC_SIZE;
>  
>  	skb = rxe_init_packet(rxe, &qp->pri_av, paylen, ack);
> @@ -934,9 +935,9 @@ static enum resp_states read_reply(struct rxe_qp *qp,
>  {
>  	struct rxe_pkt_info ack_pkt;
>  	struct sk_buff *skb;
> -	int mtu = qp->mtu;
> +	u32 mtu = qp->mtu;
>  	enum resp_states state;
> -	int payload;
> +	u32 payload;
>  	int opcode;
>  	int err;
>  	struct resp_res *res = qp->resp.res;
> @@ -982,7 +983,7 @@ static enum resp_states read_reply(struct rxe_qp *qp,
>  
>  	res->state = rdatm_res_state_next;
>  
> -	payload = min_t(int, res->read.resid, mtu);
> +	payload = min(res->read.resid, mtu);
>  
>  	skb = prepare_ack_packet(qp, &ack_pkt, opcode, payload,
>  				 res->cur_psn, AETH_ACK_UNLIMITED);
> 
> ---
> Thanks				 
> 

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

end of thread, other threads:[~2026-09-16  5:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15  6:45 [PATCH] RDMA/rxe: Fix out-of-range unsigned-to-signed conversion for RDMA message in 2GiB size Honggang LI
2026-09-15 11:49 ` Leon Romanovsky
2026-09-16  3:23   ` Honggang LI
2026-09-16  5:43     ` Leon Romanovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®