Re: [rds-devel] [External] : Re: [PATCH net] RDS/IB: validate receive completion payload length

[Gerd Rausch]

Hi Allison,

> On Oct 19, 2026, at 15:44, Allison Henderson <achender@xxxxxxxxxx> wrote:
> On Sun, 2026-10-18 at 18:32 +0000, Gerd Rausch wrote:
>> 
>> Wait. This patch was created for a hypothetical case, no?
>> Based on analysis of "Assisted-by: LLM".
> Per Documents/process/coding-assistants.rst it means an LLM contributed in
> some way, but sungbyeongchan patch had a reproducer and Shubham validated
> it under KMSAN
> 

I saw there was "a reproducer", but it is hard to tell what that thing did.
I didn't see any source-code for that reproducer.

RDS is designed to keep messages in "cp_retrans" unitl they're acknowledged by the peer.
If the connection goes down, unacknowledged messages are sent again.
It's the fundamental working principle of RDS.

What reproducer would change this behavior?

>> 
>> This patch does *not* refuse to deliver the bogus message.
>> That would have been a fine choice.
>> 
>> This patch just drops the underlying connection, in the hopes
>> that when this message gets sent again, it'll somehow be different.
> The short fragment is never linked, and the half-built inc is freed when the
> connection shuts down, so it doesn't reach the socket.
> 

Of course the message doesn't reach the socket.
The backend connection is dropped via rds_ib_conn_error() -> rds_conn_drop().

The message remains in "cp_retrans", and gets sent again.
It's the same message.
Unless there's a transport error (that CRC-32 didn't catch), or some
other non-deterministic behavior, the message will look exactly the same.

So the exact same conditional will trigger again. And again. And again.


>>> A per-message reject would be a protocol extension that both ends have to
>>> implement.  But that would be net-next material, and it couldn't help
>>> against the only peers that actually send these.  I'd be happy to review it
>>> if someone wants to take it on, but I don't think it should hold up a stable
>>> fix for a data disclosure.
>>> 
>> 
>> Again, we don't need to do that.
>> Dropping the message would have been a fine choice here.
> 
> I think dropping just the offending message, and skipping the rest
> of its fragments by sequence, would be a reasonable refinement for
> net-next.  And it would make sense to treat the header-mismatch exit
> the same way at the same time.  I'd be happy to review patches for those.
> 

Why add a wrong fix to "net" first, to then fix it in "net-next"?

Dropping an RDS connection because there was bogus payload is as useful
as a compiler closing & reloading a file again, because there was a syntax error.

The file won't look any different when you reload it.

The message that RDS sends again, won't look any different either just because
the backend connection was closed and then re-established.

> The landed fix does refuse to deliver the message though. The conn drop is
> the same policy the other exits in that function use too, so it's the right
> solution for stable.  Patches welcome on the other points though.

I guess we can agree to disagree here.

Thanks,

  Gerd