Re: [PATCH] RDMA/rxe: Fix out-of-range unsigned-to-signed conversion for RDMA message in 2GiB size

[Leon Romanovsky]

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				 
>