Re: [PATCH net v2 1/2] net: preserve rps_default_mask on failed writes

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 29 Oct 2026 15:10:36 +0000 Yingjie Wang wrote:
> rps_default_mask_sysctl() passes the live per-netns mask to
> cpumask_parse(). bitmap_parse() stores chunks as it scans from right to
> left, so a later error can leave lower chunks changed.
> rps_cpumask_housekeeping() also intersects its input in place before
> returning -EINVAL when no housekeeping CPUs remain.
> 
> Consequently, a failed write changes both the value read from the sysctl
> and the default copied to RX queues created later. On a four-CPU VM,
> after setting the mask to 1, writing zz,00000003 failed with EOVERFLOW
> but changed the mask to 3. Writing a mask containing only isolated CPUs
> failed with EINVAL but changed it to 0. No kernel modification or error
> injection was needed to reproduce either case.
> 
> Parse and validate a temporary mask before taking the mutex. After both
> steps succeed, take the mutex and replace the live pointer. Readers
> access and consume the pointer while holding the same mutex, so the
> replaced mask can be freed after dropping the mutex.
> 
> The rps_default_mask selftest failed on the base kernel and passed with
> the fix. The fixed kernel also kept the mask at 1 for the overflow and
> housekeeping failure cases, while valid writes of 2 and 0 still
> succeeded.
> 
> The issue was found while fuzzing sysctl write error paths.
> 
> Fixes: 605cfa1b1090 ("net: introduce default_rps_mask netns attribute")
> Reviewed-by: Wang Zhan <wang.zhan@smartx.com>
> Tested-by: Wang Zhan <wang.zhan@smartx.com>
> Signed-off-by: Yingjie Wang <x-professor@outlook.com>

kzalloc(cpumask_size(), GFP_KERNEL) failure seems purely theoretical.
Please drop the Fixes tag and drop the selftest, and repost after the
merge window is over (https://netdev.bots.linux.dev/net-next.html)

> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index 473c162736d69..f25281c709346 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -105,34 +105,37 @@ static int rps_default_mask_sysctl(const struct ctl_table *table, int write,
>  				   void *buffer, size_t *lenp, loff_t *ppos)
>  {
>  	struct net *net = (struct net *)table->data;
> +	struct cpumask *new_mask = NULL;
>  	struct cpumask *mask;
>  	int err = 0;
>  
> -	mutex_lock(&rps_default_mask_mutex);
> -	mask = net->core.rps_default_mask;
>  	if (write) {
> -		if (!mask) {
> -			mask = kzalloc(cpumask_size(), GFP_KERNEL);
> -			net->core.rps_default_mask = mask;
> -		}
> -		err = -ENOMEM;
> -		if (!mask)
> -			goto done;
> +		new_mask = kzalloc(cpumask_size(), GFP_KERNEL);
> +		if (!new_mask)
> +			return -ENOMEM;
>  
> -		err = cpumask_parse(buffer, mask);
> +		err = cpumask_parse(buffer, new_mask);
>  		if (err)
> -			goto done;
> +			goto out;
>  
> -		err = rps_cpumask_housekeeping(mask);
> +		err = rps_cpumask_housekeeping(new_mask);
>  		if (err)
> -			goto done;
> +			goto out;
> +	}
> +
> +	mutex_lock(&rps_default_mask_mutex);
> +	mask = net->core.rps_default_mask;
> +	if (write) {
> +		net->core.rps_default_mask = new_mask;
> +		new_mask = mask;

nit: this could use

		swap(net->core.rps_default_mask, new_mask);

>  	} else {
>  		err = dump_cpumask(buffer, lenp, ppos,
>  				   mask ?: cpu_none_mask);
>  	}
>  
> -done:
>  	mutex_unlock(&rps_default_mask_mutex);
> +out:
> +	kfree(new_mask);
>  	return err;
>  }
>  
-- 
pw-bot: cr