Re: [PATCH net-next 07/14] mptcp: annotate data-races around pernet sysctls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] mptcp: torn read in mptcp_get_path_manager() [1]
--

commit 320fdfa473078fc92c3d7c8719ab1b53608f74ae
Author: Jiayuan Chen <jiayuan.chen@linux.dev>

mptcp: annotate data-races around pernet sysctls

This commit adds READ_ONCE() and WRITE_ONCE() annotations to pernet sysctl
fields and their accessors in net/mptcp/ctrl.c to avoid lockless data races.

> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index 63c5747f0f63c..164f79b9deb0e 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c

[ ... ]

> @@ -230,7 +230,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: Medium]
This problem wasn't introduced by this patch, but can concurrent sysctl
accesses cause a torn read of pernet->path_manager [1]?

proc_path_manager() updates the inline path_manager buffer in place via
mptcp_set_path_manager() without synchronization against concurrent readers:

CPU0                              CPU1
proc_path_manager()               proc_path_manager()
  mptcp_set_path_manager()
    strscpy(path_manager, ...)
                                    strscpy(pm_name, *path_manager, ...)
                                      <-- torn read

A concurrent sysctl read or a caller of mptcp_get_path_manager() can
therefore observe a partially written path manager name.

[1] https://sashiko.dev/#/bug/linux-276f38f8-a836-44bc-9e44-c537511752aa

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109025425.328445-1-jiayuan.chen@linux.dev?part=7