Re: [PATCH mptcp-next v12 06/10] mptcp: align FIN handling with TCP via SOCK_DONE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Memory barrier API violation: smp_mb__before_atomic() is used before a non-atomic bitwise operation, which fails to emit a hardware memory barrier on architectures like LoongArch, allowing store-store reordering and breaking synchronization.
--

commit ed77cdb78ed1ea97ec780c81ea48c8eba994517c
Author: Geliang Tang <geliang@kernel.org>
Subject: mptcp: align FIN handling with TCP via SOCK_DONE

This commit aligns MPTCP's FIN handling with TCP by using the SOCK_DONE flag 
to mark FIN reception alongside RCV_SHUTDOWN. It updates mptcp_inq_hint() and 
mptcp_recvmsg() to check SOCK_DONE, ensuring the FIN handling only triggers 
on a real DATA_FIN.

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 9bcca31501e7d..f970b92429726 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -736,6 +736,7 @@ static void mptcp_check_data_fin(struct sock *sk)
>  
>  		WRITE_ONCE(sk->sk_shutdown, sk->sk_shutdown | RCV_SHUTDOWN);
>  		smp_mb__before_atomic(); /* SHUTDOWN must be visible first */
> +		sock_set_flag(sk, SOCK_DONE);

[Severity: High]
Does this memory barrier adequately protect the store to SOCK_DONE on weakly
ordered architectures?

According to the atomic operations documentation, smp_mb__before_atomic() must
only be used immediately before atomic RMW operations. Since sock_set_flag()
uses __set_bit() which is non-atomic, the barrier might not emit a hardware
memory barrier on architectures like LoongArch (where it is defined merely as
a compiler barrier).

If the CPU reorders the non-atomic stores, SOCK_DONE could become visible
before the ack_seq update in mptcp_check_data_fin(). A concurrent lockless
reader in mptcp_inq_hint() executing smp_rmb() might see the new SOCK_DONE
flag but read a stale ack_seq.

Could this cause hint_val to undercount by 1 byte and potentially hang
TLS/BPF consumers waiting for the final byte? Would it be safer to use
smp_wmb() or smp_mb() here to guarantee store-store ordering?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788171508.git.tanggeliang@kylinos.cn?part=6