mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net-next v3] net: preserve rps_default_mask on failed writes
@ 2026-11-08 13:19 Yingjie Wang
  2026-11-09 15:17 ` Wang Zhan
  0 siblings, 1 reply; 2+ messages in thread
From: Yingjie Wang @ 2026-11-08 13:19 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, horms, wang.zhan, kuniyu, linux-kernel

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, swap it with the live mask under rps_default_mask_mutex.
Readers access and consume the pointer while holding the same mutex, so
the replaced mask can be freed after dropping the mutex.

The issue was found while fuzzing sysctl write error paths. With the fix,
the overflow and housekeeping failures both retained the mask at 1,
while valid writes of 2 and 0 still succeeded. The existing
rps_default_mask selftest also passed.

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>
---
Changes since v2:
- Use swap() to publish the new mask, as suggested by Jakub Kicinski.
- Drop the Fixes tag and the new selftest patch.
- Retarget to net-next and rebase onto net-next/main at fa5196392691.

v2: https://lore.kernel.org/r/cover.1793286626.git.x-professor@outlook.com

 net/core/sysctl_net_core.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 9226b96f2e89..462d2d63f0cf 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -105,34 +105,36 @@ 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) {
+		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;
 }
 

base-commit: fa5196392691051963726787944db872420dfa75
-- 
2.43.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH net-next v3] net: preserve rps_default_mask on failed writes
  2026-11-08 13:19 [PATCH net-next v3] net: preserve rps_default_mask on failed writes Yingjie Wang
@ 2026-11-09 15:17 ` Wang Zhan
  0 siblings, 0 replies; 2+ messages in thread
From: Wang Zhan @ 2026-11-09 15:17 UTC (permalink / raw)
  To: netdev, Yingjie Wang
  Cc: Wang Zhan, davem, edumazet, kuba, pabeni, horms, kuniyu, linux-kernel

On Sun, 08 Nov 2026 13:19:12 +0000 Yingjie Wang wrote:
Two nits, if you happen to respin:

> @@ -105,34 +105,36 @@ 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;

nit: use net->core.rps_default_mask directly in the read path and drop
the variable; there are no other users.

> @@ -105,34 +105,36 @@ 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) {
> +		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);

nit: move this up to the closing brace and add a blank line before out:, so
the unlock is visibly paired with the lock.

Thanks

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-11-09 15:17 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-08 13:19 [PATCH net-next v3] net: preserve rps_default_mask on failed writes Yingjie Wang
2026-11-09 15:17 ` Wang Zhan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®