[PATCH net] net/rds: ib: drop fragments shorter than the header-declared length

[PATCH net] net/rds: ib: drop fragments shorter than the header-declared length

[Shubham Antil]

rds_ib_process_recv() accepts an incoming RDS/IB fragment once the
receive completion reports at least an RDS header
(data_len >= sizeof(struct rds_header)).  It then trusts the
header-declared total message length h_len: for the first fragment of
a message it stores be32_to_cpu(hdr->h_len) in ic->i_recv_data_rem,
and rds_ib_inc_copy_to_user() later copies up to h_len bytes from the
fragment pages to userspace on recvmsg().

The number of payload bytes actually received into the fragment page is
data_len (after subtracting the header), but it is never checked against
the amount the fragment is accounted to contribute to the message,
min(i_recv_data_rem, RDS_FRAG_SIZE).  A fragment whose header advertises
a larger h_len than the payload it delivers is still linked onto the
reassembly list.  The fragment page comes from the per-CPU receive cache
and is not zeroed, so rds_ib_inc_copy_to_user() then copies up to h_len
bytes to the PF_RDS reader, including the uninitialized tail the receive
never wrote.

Reject a fragment that carries fewer payload bytes than it is accounted
to contribute before linking it onto the reassembly list.

The issue is reproducible under KMSAN with two hosts over rdma_rxe
(Soft-RoCE); the same reproducer confirms the fix stops it.

Fixes: 1e23b3ee0e94 ("RDS/IB: Receive datagrams via IB")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Shubham Antil <shubham@octane.security>
---
 net/rds/ib_recv.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
index bd6cb3ffa..fee77b6d7 100644
--- a/net/rds/ib_recv.c
+++ b/net/rds/ib_recv.c
@@ -949,6 +949,15 @@ static void rds_ib_process_recv(struct rds_connection *conn,
 		}
 	}
 
+	/* h_len must be backed by the payload actually received (data_len),
+	 * else the unwritten frag-page tail is copied to userspace.
+	 */
+	if (data_len < min_t(u32, ic->i_recv_data_rem, RDS_FRAG_SIZE)) {
+		rds_ib_conn_error(conn,
+			"fragment shorter than header-declared length; forcing reconnect\n");
+		goto done;
+	}
+
 	list_add_tail(&recv->r_frag->f_item, &ibinc->ii_frags);
 	recv->r_frag = NULL;
 
-- 
2.43.0

Re: [PATCH net] net/rds: ib: drop fragments shorter than the header-declared length

[sashiko-bot]

> rds_ib_process_recv() accepts an incoming RDS/IB fragment once the
> receive completion reports at least an RDS header
> (data_len >= sizeof(struct rds_header)).  It then trusts the
> header-declared total message length h_len: for the first fragment of
> a message it stores be32_to_cpu(hdr->h_len) in ic->i_recv_data_rem,
> and rds_ib_inc_copy_to_user() later copies up to h_len bytes from the
> fragment pages to userspace on recvmsg().
> 
> The number of payload bytes actually received into the fragment page is
> data_len (after subtracting the header), but it is never checked against
> the amount the fragment is accounted to contribute to the message,
> min(i_recv_data_rem, RDS_FRAG_SIZE).  A fragment whose header advertises
> a larger h_len than the payload it delivers is still linked onto the
> reassembly list.  The fragment page comes from the per-CPU receive cache
> and is not zeroed, so rds_ib_inc_copy_to_user() then copies up to h_len
> bytes to the PF_RDS reader, including the uninitialized tail the receive
> never wrote.
> 
> Reject a fragment that carries fewer payload bytes than it is accounted
> to contribute before linking it onto the reassembly list.
> 
> The issue is reproducible under KMSAN with two hosts over rdma_rxe
> (Soft-RoCE); the same reproducer confirms the fix stops it.
> 
> Fixes: 1e23b3ee0e94 ("RDS/IB: Receive datagrams via IB")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Shubham Antil <shubham@octane.security>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006125720.81227-1-shubham@octane.security?part=1

Re: [PATCH net] net/rds: ib: drop fragments shorter than the header-declared length

[Allison Henderson]

On Tue, 2026-10-06 at 18:27 +0530, Shubham Antil wrote:
> rds_ib_process_recv() accepts an incoming RDS/IB fragment once the
> receive completion reports at least an RDS header
> (data_len >= sizeof(struct rds_header)).  It then trusts the
> header-declared total message length h_len: for the first fragment of
> a message it stores be32_to_cpu(hdr->h_len) in ic->i_recv_data_rem,
> and rds_ib_inc_copy_to_user() later copies up to h_len bytes from the
> fragment pages to userspace on recvmsg().
> 
> The number of payload bytes actually received into the fragment page is
> data_len (after subtracting the header), but it is never checked against
> the amount the fragment is accounted to contribute to the message,
> min(i_recv_data_rem, RDS_FRAG_SIZE).  A fragment whose header advertises
> a larger h_len than the payload it delivers is still linked onto the
> reassembly list.  The fragment page comes from the per-CPU receive cache
> and is not zeroed, so rds_ib_inc_copy_to_user() then copies up to h_len
> bytes to the PF_RDS reader, including the uninitialized tail the receive
> never wrote.
> 
> Reject a fragment that carries fewer payload bytes than it is accounted
> to contribute before linking it onto the reassembly list.
> 
> The issue is reproducible under KMSAN with two hosts over rdma_rxe
> (Soft-RoCE); the same reproducer confirms the fix stops it.
> 
> Fixes: 1e23b3ee0e94 ("RDS/IB: Receive datagrams via IB")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Shubham Antil <shubham@octane.security>

Hi Shubham,

Thanks for working on this.  This patch looks good to me, you can add my rvb:
Reviewed-by: Allison Henderson <achender@kernel.org>

Also, this bug was found and reported privately by another contributor, whom I had
counseled to send a patch publicly before I had noticed this one.  
https://lore.kernel.org/netdev/20261006205204.1322102-1-tjdqudcks0424@naver.com/

I find that patches equivalent, and this patch was posted first. But I would like to apply
the reported by tag since sungbyeongchan was the first to report it.
Reported-by: sungbyeongchan <tjdqudcks0424@naver.com>

Thank you both for working on this bug!
Allison

> ---
>  net/rds/ib_recv.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
> index bd6cb3ffa..fee77b6d7 100644
> --- a/net/rds/ib_recv.c
> +++ b/net/rds/ib_recv.c
> @@ -949,6 +949,15 @@ static void rds_ib_process_recv(struct rds_connection *conn,
>  		}
>  	}
>  
> +	/* h_len must be backed by the payload actually received (data_len),
> +	 * else the unwritten frag-page tail is copied to userspace.
> +	 */
> +	if (data_len < min_t(u32, ic->i_recv_data_rem, RDS_FRAG_SIZE)) {
> +		rds_ib_conn_error(conn,
> +			"fragment shorter than header-declared length; forcing reconnect\n");
> +		goto done;
> +	}
> +
>  	list_add_tail(&recv->r_frag->f_item, &ibinc->ii_frags);
>  	recv->r_frag = NULL;
>  

Re: [PATCH net] net/rds: ib: drop fragments shorter than the header-declared length

[Shubham Antil]

Hi Allison,

Thanks very much — I'll carry your Reviewed-by on the next revision:

Reviewed-by: Allison Henderson <achender@kernel.org>

And no objection at all to crediting the earlier private report:

Reported-by: sungbyeongchan <tjdqudcks0424@naver.com>

This bug was found and reported by our team at Octane Security. Would
it be okay if we add Reported-by tags for the people on our side who
reported it along with the first reporter in the V2 patch?

Reported-by: Shubham Antil <shubham@octane.security>
Reported-by: Giovanni Vignone <gio@octane.security>
Reported-by: Robert van Eijk <robert@octane.security>
Reported-by: Paolo Gentry <paolo@octane.security>

I'm happy to send a v2 with everything folded in - your Reviewed-by,
sungbyeongchan's Reported-by and ours. Just let me know which you
prefer.

Thanks again,
Shubham Antil

On Wed, Oct 7, 2026 at 6:43 AM Allison Henderson <achender@kernel.org> wrote:
>
> On Tue, 2026-10-06 at 18:27 +0530, Shubham Antil wrote:
> > rds_ib_process_recv() accepts an incoming RDS/IB fragment once the
> > receive completion reports at least an RDS header
> > (data_len >= sizeof(struct rds_header)).  It then trusts the
> > header-declared total message length h_len: for the first fragment of
> > a message it stores be32_to_cpu(hdr->h_len) in ic->i_recv_data_rem,
> > and rds_ib_inc_copy_to_user() later copies up to h_len bytes from the
> > fragment pages to userspace on recvmsg().
> >
> > The number of payload bytes actually received into the fragment page is
> > data_len (after subtracting the header), but it is never checked against
> > the amount the fragment is accounted to contribute to the message,
> > min(i_recv_data_rem, RDS_FRAG_SIZE).  A fragment whose header advertises
> > a larger h_len than the payload it delivers is still linked onto the
> > reassembly list.  The fragment page comes from the per-CPU receive cache
> > and is not zeroed, so rds_ib_inc_copy_to_user() then copies up to h_len
> > bytes to the PF_RDS reader, including the uninitialized tail the receive
> > never wrote.
> >
> > Reject a fragment that carries fewer payload bytes than it is accounted
> > to contribute before linking it onto the reassembly list.
> >
> > The issue is reproducible under KMSAN with two hosts over rdma_rxe
> > (Soft-RoCE); the same reproducer confirms the fix stops it.
> >
> > Fixes: 1e23b3ee0e94 ("RDS/IB: Receive datagrams via IB")
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Shubham Antil <shubham@octane.security>
>
> Hi Shubham,
>
> Thanks for working on this.  This patch looks good to me, you can add my rvb:
> Reviewed-by: Allison Henderson <achender@kernel.org>
>
> Also, this bug was found and reported privately by another contributor, whom I had
> counseled to send a patch publicly before I had noticed this one.
> https://lore.kernel.org/netdev/20261006205204.1322102-1-tjdqudcks0424@naver.com/
>
> I find that patches equivalent, and this patch was posted first. But I would like to apply
> the reported by tag since sungbyeongchan was the first to report it.
> Reported-by: sungbyeongchan <tjdqudcks0424@naver.com>
>
> Thank you both for working on this bug!
> Allison
>
> > ---
> >  net/rds/ib_recv.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
> > index bd6cb3ffa..fee77b6d7 100644
> > --- a/net/rds/ib_recv.c
> > +++ b/net/rds/ib_recv.c
> > @@ -949,6 +949,15 @@ static void rds_ib_process_recv(struct rds_connection *conn,
> >               }
> >       }
> >
> > +     /* h_len must be backed by the payload actually received (data_len),
> > +      * else the unwritten frag-page tail is copied to userspace.
> > +      */
> > +     if (data_len < min_t(u32, ic->i_recv_data_rem, RDS_FRAG_SIZE)) {
> > +             rds_ib_conn_error(conn,
> > +                     "fragment shorter than header-declared length; forcing reconnect\n");
> > +             goto done;
> > +     }
> > +
> >       list_add_tail(&recv->r_frag->f_item, &ibinc->ii_frags);
> >       recv->r_frag = NULL;
> >
>

Re: [PATCH net] net/rds: ib: drop fragments shorter than the header-declared length

[Allison Henderson]

On Wed, 2026-10-07 at 07:13 +0530, Shubham Antil wrote:
> Hi Allison,
> 
> Thanks very much — I'll carry your Reviewed-by on the next revision:
> 
> Reviewed-by: Allison Henderson <achender@kernel.org>
> 
> And no objection at all to crediting the earlier private report:
> 
> Reported-by: sungbyeongchan <tjdqudcks0424@naver.com>
> 
> This bug was found and reported by our team at Octane Security. Would
> it be okay if we add Reported-by tags for the people on our side who
> reported it along with the first reporter in the V2 patch?
> 
> Reported-by: Shubham Antil <shubham@octane.security>
> Reported-by: Giovanni Vignone <gio@octane.security>
> Reported-by: Robert van Eijk <robert@octane.security>
> Reported-by: Paolo Gentry <paolo@octane.security>

Sure, that's fine.  Thank you!
Allison

> 
> I'm happy to send a v2 with everything folded in - your Reviewed-by,
> sungbyeongchan's Reported-by and ours. Just let me know which you
> prefer.
> 
> Thanks again,
> Shubham Antil
> 
> On Wed, Oct 7, 2026 at 6:43 AM Allison Henderson <achender@kernel.org> wrote:
> > 
> > On Tue, 2026-10-06 at 18:27 +0530, Shubham Antil wrote:
> > > rds_ib_process_recv() accepts an incoming RDS/IB fragment once the
> > > receive completion reports at least an RDS header
> > > (data_len >= sizeof(struct rds_header)).  It then trusts the
> > > header-declared total message length h_len: for the first fragment of
> > > a message it stores be32_to_cpu(hdr->h_len) in ic->i_recv_data_rem,
> > > and rds_ib_inc_copy_to_user() later copies up to h_len bytes from the
> > > fragment pages to userspace on recvmsg().
> > > 
> > > The number of payload bytes actually received into the fragment page is
> > > data_len (after subtracting the header), but it is never checked against
> > > the amount the fragment is accounted to contribute to the message,
> > > min(i_recv_data_rem, RDS_FRAG_SIZE).  A fragment whose header advertises
> > > a larger h_len than the payload it delivers is still linked onto the
> > > reassembly list.  The fragment page comes from the per-CPU receive cache
> > > and is not zeroed, so rds_ib_inc_copy_to_user() then copies up to h_len
> > > bytes to the PF_RDS reader, including the uninitialized tail the receive
> > > never wrote.
> > > 
> > > Reject a fragment that carries fewer payload bytes than it is accounted
> > > to contribute before linking it onto the reassembly list.
> > > 
> > > The issue is reproducible under KMSAN with two hosts over rdma_rxe
> > > (Soft-RoCE); the same reproducer confirms the fix stops it.
> > > 
> > > Fixes: 1e23b3ee0e94 ("RDS/IB: Receive datagrams via IB")
> > > Assisted-by: Claude:claude-opus-4-8
> > > Signed-off-by: Shubham Antil <shubham@octane.security>
> > 
> > Hi Shubham,
> > 
> > Thanks for working on this.  This patch looks good to me, you can add my rvb:
> > Reviewed-by: Allison Henderson <achender@kernel.org>
> > 
> > Also, this bug was found and reported privately by another contributor, whom I had
> > counseled to send a patch publicly before I had noticed this one.
> > https://lore.kernel.org/netdev/20261006205204.1322102-1-tjdqudcks0424@naver.com/
> > 
> > I find that patches equivalent, and this patch was posted first. But I would like to apply
> > the reported by tag since sungbyeongchan was the first to report it.
> > Reported-by: sungbyeongchan <tjdqudcks0424@naver.com>
> > 
> > Thank you both for working on this bug!
> > Allison
> > 
> > > ---
> > >  net/rds/ib_recv.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
> > > index bd6cb3ffa..fee77b6d7 100644
> > > --- a/net/rds/ib_recv.c
> > > +++ b/net/rds/ib_recv.c
> > > @@ -949,6 +949,15 @@ static void rds_ib_process_recv(struct rds_connection *conn,
> > >               }
> > >       }
> > > 
> > > +     /* h_len must be backed by the payload actually received (data_len),
> > > +      * else the unwritten frag-page tail is copied to userspace.
> > > +      */
> > > +     if (data_len < min_t(u32, ic->i_recv_data_rem, RDS_FRAG_SIZE)) {
> > > +             rds_ib_conn_error(conn,
> > > +                     "fragment shorter than header-declared length; forcing reconnect\n");
> > > +             goto done;
> > > +     }
> > > +
> > >       list_add_tail(&recv->r_frag->f_item, &ibinc->ii_frags);
> > >       recv->r_frag = NULL;
> > > 
> >