Re: [PATCH mptcp-next v2 3/5] mptcp: use READ_ONCE() over sysctls

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Concurrent sysctl writes or sysctl updates racing with socket creation can leave the socket's `pm_type` and `ops` mismatched, breaking MPTCP path management.
--

--- Patch [3]: [PATCH mptcp-next v2 3/5] mptcp: use READ_ONCE() over sysctls ---
commit 77c9e4aa4cdd5f6c57cc4cab253d46fd0cf2dfb2
Author: Matthieu Baerts (NGI0) <[email protected]>

mptcp: use READ_ONCE() over sysctls

This commit adds READ_ONCE() and WRITE_ONCE() macros around sysctl
variables in the MPTCP networking code to avoid KCSAN issues. It also
ensures the pm_type is written with WRITE_ONCE() in proc_path_manager().

Closes: https://lore.kernel.org/CANn89iL=os-60kDKqMDdyiXuPF5CG=eejS0vmthwpDGXz_Bp8A@mail.gmail.com

> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index c0481b09c1a17..733706f06f1b1 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c

[ ... ]

> @@ -251,7 +251,7 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
>  				pm_type = MPTCP_PM_TYPE_KERNEL;
>  			else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0)
>  				pm_type = MPTCP_PM_TYPE_USERSPACE;
> -			pernet->pm_type = pm_type;
> +			WRITE_ONCE(pernet->pm_type, pm_type);

[Severity: High]
This is a pre-existing issue, but can this sequential update race with socket
creation and leave the socket's pm_type and ops mismatched?

If an administrator writes to the sysctl while a new socket is being created,
could mptcp_pm_data_reset() in net/mptcp/pm.c interleave like this:

CPU1 (sysctl write)
proc_path_manager()
    mptcp_set_path_manager(pernet, pm_name); // Updates pernet->path_manager

CPU2 (socket creation)
mptcp_pm_data_reset()
    pm_type = mptcp_get_pm_type(net);        // Reads old pm_type
    ...
    mptcp_get_path_manager(net, pm_name);    // Reads new path_manager

CPU1 (sysctl write resumes)
proc_path_manager()
    WRITE_ONCE(pernet->pm_type, pm_type);    // Updates pernet->pm_type

Would this leave the new socket corrupted with a contradictory path manager
state, breaking MPTCP path management logic?

>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3