Re: Patch "mptcp: annotate data-races around subflow->fully_established" has been added to the 6.12-stable tree

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] mptcp: build failure due to READ_ONCE() on bit-field
--

commit 2cede4d0675228e6db5b209f56d2a298cf7cb2e9
Author: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>
Subject: Patch "mptcp: annotate data-races around subflow->fully_established" has been added to the 6.12-stable tree

This commit annotates data races around the fully_established flag in the
mptcp subflow context. It aims to convert the flag from a bit-field to a
boolean type to safely use READ_ONCE() and WRITE_ONCE().

Link: https://patch.msgid.link/20241021-net-next-mptcp-misc-6-13-v1-2-1ef02746504a@kernel.org

>     Additionally, we make a crucial change: convert the subflow's
>     'fully_established' from 'bit_field' to 'bool' type. This is
>     necessary because methods for avoiding data races don't work well
>     with 'bit_field'. Specifically, the 'READ_ONCE' needs to know
>     the size of the variable being accessed, which is not supported in
>     'bit_field'. Also, 'test_bit' expect the address of 'bit_field'.

[Severity: Low]
Was the hunk converting this field to a boolean dropped during the backport?
It appears fully_established is still defined as a bit-field in
struct mptcp_subflow_context.

Additionally, the write side update was missed in the backport:

net/mptcp/subflow.c:__mptcp_subflow_fully_established() {
    ...
	subflow->fully_established = 1;
    ...
}

This lacks the WRITE_ONCE() annotation and continues to perform an unsafe
read-modify-write on the shared bit-field word.

> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 370c3836b7712..1603b3702e220 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -461,7 +461,7 @@ static bool mptcp_established_options_mp(struct sock *sk, struct sk_buff *skb,
>  		return false;
>  
>  	/* MPC/MPJ needed only on 3rd ack packet, DATA_FIN and TCP shutdown take precedence */
> -	if (subflow->fully_established || snd_data_fin_enable ||
> +	if (READ_ONCE(subflow->fully_established) || snd_data_fin_enable ||

[Severity: Low]
Does this trigger a compiler error? Since the struct modification was missed
and fully_established remains a bit-field, applying READ_ONCE() here attempts
to take the address of a bit-field, which is syntactically invalid in C.

[ ... ]

> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 568a72702b080..e9dba0708ee70 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -532,7 +532,7 @@ struct mptcp_subflow_context {
>  		is_mptfo : 1,	    /* subflow is doing TFO */
>  		close_event_done : 1,       /* has done the post-closed part */
>  		mpc_drop : 1,	    /* the MPC option has been dropped in a rtx */
> -		__unused : 8;
> +		__unused : 9;

[Severity: Low]
Does this increase the total bit-field group to 33 bits, causing a boundary
overflow? 

Since fully_established wasn't actually removed from the bit-field group in
this backport, incrementing the unused bits to 9 pushes the total beyond a
32-bit word size.

>  	bool	data_avail;
>  	bool	scheduled;
>  	bool	pm_listener;	    /* a listener managed by the kernel PM? */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/2026092037-islamic-default-0aca@gregkh?part=1