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@xxxxxxxxxx>

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

Reported-by: sungbyeongchan <tjdqudcks0424@xxxxxxxxx>

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@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> 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@xxxxxxxxx/
>
> 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@xxxxxxxxx>
>
> 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;
> >
>