Re: [PATCH mptcp-net] mptcp: fix data re-injection from stale subflow

[Matthieu Baerts <matttbe@kernel.org>]

Hi Paolo, Mat,

On 23/01/2024 22:03, Paolo Abeni wrote:
> When the MPTCP PM detects that a subflow is stale, all the packet
> scheduler must re-inject all the mptcp-level unacked data. To avoid
> acquiring unneeded locks, it first try to check if any unacked data
> is present at all in the RTX queue, but such check is currently
> broken, as it uses TCP-specific helper on an MPTCP socket.
> 
> Funnily enough fuzzers and static checkers are happy, as the accessed
> memory still belongs to the mptcp_sock struct, and even from a
> functional perspective the recovery completed successfully, as
> the short-cut test always failed.
> 
> A recent unrelated TCP change - commit d5fed5addb2b ("tcp: reorganize
> tcp_sock fast path variables") - exposed the issue, as the tcp field
> reorganization makes the mptcp code always skip the re-inection.
> 
> Fix the issue dropping the bogus call: we are on a slow path, the early
> optimization proved once again to be evil.
> 
> Fixes: 1e1d9d6f119c ("mptcp: handle pending data on closed subflow")
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/468
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Thank you for the patch and the review!

Any ideas how to prevent such issues? Changing the signature of the
TCP-specific functions? Adding a custom check for our CI on our side?

New patches for t/upstream-net and t/upstream:
- 359966180a29: mptcp: fix data re-injection from stale subflow
- Results: d2cf5db9b409..ca82f8ccb3cc (export-net)
- Results: 171302925a80..4b8f1ec22243 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20240130T115340
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20240130T115340

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.