Re: [PATCH net] ipvs: fix races around est_mutex and est_cpulist

	Hello,

On Mon, 20 Apr 2026, Julian Anastasov wrote:

> Sashiko reports for races and possible crash around
> the usage of est_cpulist_valid and sysctl_est_cpulist.
> The problem is that we do not lock est_mutex in some
> places which can lead to wrong write ordering and
> as result problems when calling cpumask_weight()
> and cpumask_empty().
> 
> Fix them by moving the est_max_threads read/write under
> locked est_mutex. Do the same for one ip_vs_est_reload_start()
> call to protect the cpumask_empty() usage of sysctl_est_cpulist.
> 
> Link: https://sashiko.dev/#/patchset/20260331165015.2777765-1-longman%40redhat.com
> Fixes: f0be83d54217 ("ipvs: add est_cpulist and est_nice sysctl vars")
> Signed-off-by: Julian Anastasov <[email protected]>

	According to Sashiko, this patch needs more
work, I'll send new version when I'm ready...

pw-bot: changes-requested


> ---
> 
>  Note that this patch complements v2 of patchset from 31-MAR-26
>  "ipvs: Fix incorrect use of HK_TYPE_KTHREAD housekeeping cpumask"
>  and can be applied before it to avoid the bad AI reviews:
> 
>  https://lore.kernel.org/all/[email protected]/
> 
>  net/netfilter/ipvs/ip_vs_ctl.c |  5 +++++
>  net/netfilter/ipvs/ip_vs_est.c | 22 +++++++++++++++-------
>  2 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index caec516856e9..8778e174ef56 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1812,11 +1812,16 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u,
>  	*svc_p = svc;
>  
>  	if (!READ_ONCE(ipvs->enable)) {
> +		mutex_lock(&ipvs->est_mutex);
> +
>  		/* Now there is a service - full throttle */
>  		WRITE_ONCE(ipvs->enable, 1);
>  
> +		ipvs->est_max_threads = ip_vs_est_max_threads(ipvs);
> +
>  		/* Start estimation for first time */
>  		ip_vs_est_reload_start(ipvs);
> +		mutex_unlock(&ipvs->est_mutex);
>  	}
>  
>  	return 0;
> diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c
> index 433ba3cab58c..6c9981d5611e 100644
> --- a/net/netfilter/ipvs/ip_vs_est.c
> +++ b/net/netfilter/ipvs/ip_vs_est.c
> @@ -68,6 +68,10 @@
>      and the limit of estimators per kthread
>    - est_add_ktid: ktid where to add new ests, can point to empty slot where
>      we should add kt data
> +  - data protected by service_mutex: est_temp_list, est_add_ktid
> +  - data protected by est_mutex: est_kt_count, est_kt_arr, est_max_threads,
> +    sysctl_est_cpulist, est_cpulist_valid, sysctl_est_nice, est_stopped,
> +    sysctl_run_estimation
>   */
>  
>  static struct lock_class_key __ipvs_est_key;
> @@ -229,6 +233,8 @@ static int ip_vs_estimation_kthread(void *data)
>  /* Schedule stop/start for kthread tasks */
>  void ip_vs_est_reload_start(struct netns_ipvs *ipvs)
>  {
> +	lockdep_assert_held(&ipvs->est_mutex);
> +
>  	/* Ignore reloads before first service is added */
>  	if (!READ_ONCE(ipvs->enable))
>  		return;
> @@ -304,12 +310,17 @@ static int ip_vs_est_add_kthread(struct netns_ipvs *ipvs)
>  	void *arr = NULL;
>  	int i;
>  
> -	if ((unsigned long)ipvs->est_kt_count >= ipvs->est_max_threads &&
> -	    READ_ONCE(ipvs->enable) && ipvs->est_max_threads)
> -		return -EINVAL;
> -
>  	mutex_lock(&ipvs->est_mutex);
>  
> +	/* Allow kt 0 data to be created before the services are added
> +	 * and limit the kthreads when services are present.
> +	 */
> +	if ((unsigned long)ipvs->est_kt_count >= ipvs->est_max_threads &&
> +	    READ_ONCE(ipvs->enable) && ipvs->est_max_threads) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
>  	for (i = 0; i < id; i++) {
>  		if (!ipvs->est_kt_arr[i])
>  			break;
> @@ -485,9 +496,6 @@ int ip_vs_start_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats)
>  	struct ip_vs_estimator *est = &stats->est;
>  	int ret;
>  
> -	if (!ipvs->est_max_threads && READ_ONCE(ipvs->enable))
> -		ipvs->est_max_threads = ip_vs_est_max_threads(ipvs);
> -
>  	est->ktid = -1;
>  	est->ktrow = IPVS_EST_NTICKS - 1;	/* Initial delay */
>  
> -- 
> 2.53.0

Regards

--
Julian Anastasov <[email protected]>