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

[Allison Henderson <achender@kernel.org>]

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;
> > > 
> >