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 <redacted>
Tested-by: Wang Zhan <redacted>
Signed-off-by: Yingjie Wang <redacted>
---
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