The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] sched/debug: Validate writes to the scan_size_mb debugfs knob
@ 2026-08-10  8:18 Zhan Xusheng
  2026-08-10 13:57 ` Chen Yu
  0 siblings, 1 reply; 2+ messages in thread
From: Zhan Xusheng @ 2026-08-10  8:18 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot
  Cc: Mel Gorman, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Valentin Schneider, K Prateek Nayak, zhanxusheng, linux-kernel

sysctl_numa_balancing_scan_size is used as a divisor, so two kinds of
write to scan_size_mb kill the machine.

Zero makes task_scan_min() divide by it:

	windows = MAX_SCAN_WINDOW / scan_size;

  # echo 0 > /sys/kernel/debug/sched/numa_balancing/scan_size_mb

  Oops: divide error: 0000 [#1] SMP PTI
  RIP: 0010:task_scan_max+0x30/0x1a0
  RAX: 0000000000000a00 RCX: 0000000000000000
  Call Trace:
   init_numa_balancing+0xe0/0x200
   __sched_fork+0x13b/0x180
   sched_fork+0x12/0x1d0
   copy_process+0xdea/0x2370
   kernel_clone+0xd6/0x4a0
   __do_sys_clone3+0xf4/0x140

task_scan_min() is inlined there.  The trigger is thread creation rather
than fork, since init_numa_balancing() returns early for a new address
space and only reaches task_scan_max() for a CLONE_VM child.

A multiple of 2^24 (with 4K pages) is the second kind.  MB_TO_PAGES()
shifts an unsigned int, so the result wraps to zero and
task_nr_scan_windows() divides by that instead:

  # echo 16777216 > /sys/kernel/debug/sched/numa_balancing/scan_size_mb

  Oops: divide error: 0000 [#1] SMP PTI
  RIP: 0010:task_nr_scan_windows.isra.0+0x5c/0x70

The move to debugfs kept validation where a bad value was harmful:
tunable_scaling gained sched_scaling_fops, so an out-of-range write there
returns -EINVAL rather than being stored.  scan_size_mb went to a plain
debugfs_create_u32() instead, and the

	.procname	= "numa_balancing_scan_size_mb",
	.extra1		= SYSCTL_ONE,

it used to have was not carried over, so zero became storable.  No upper
bound was ever enforced, so the wrap predates that commit.

Follow sched_scaling_fops and refuse both instead of storing them.  The
knob is described in Documentation/scheduler/sched-debug.rst as one of the
files controlling the scan rate, so returning -EINVAL is friendlier than
quietly substituting a different value.

Checked in a 2-node qemu guest: on an unpatched 7.2.0-rc6 the two writes
above panic at the two RIPs shown, and with this patch writes of 0, 2^24
and 3*2^24 all get -EINVAL while a write of 512 still takes effect.

Fixes: 8a99b6833c88 ("sched: Move SCHED_DEBUG sysctl to debugfs")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 kernel/sched/debug.c | 51 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 40584b27ea0c..e09fe94e7b9c 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -210,6 +210,55 @@ static const struct file_operations sched_scaling_fops = {
 	.release	= single_release,
 };
 
+#ifdef CONFIG_NUMA_BALANCING
+/*
+ * scan_size_mb is fed to MB_TO_PAGES() and then used as a divisor by
+ * task_nr_scan_windows() and task_scan_min().  Zero divides by zero, and a
+ * value big enough to overflow the 32-bit shift in MB_TO_PAGES() wraps to
+ * zero and does the same, so refuse both rather than store them.
+ */
+#define NUMA_SCAN_SIZE_MB_MAX	(UINT_MAX >> (20 - PAGE_SHIFT))
+
+static ssize_t sched_numa_scan_size_write(struct file *filp,
+					  const char __user *ubuf,
+					  size_t cnt, loff_t *ppos)
+{
+	unsigned int mb;
+	int ret;
+
+	ret = kstrtouint_from_user(ubuf, cnt, 10, &mb);
+	if (ret)
+		return ret;
+
+	if (!mb || mb > NUMA_SCAN_SIZE_MB_MAX)
+		return -EINVAL;
+
+	sysctl_numa_balancing_scan_size = mb;
+
+	*ppos += cnt;
+	return cnt;
+}
+
+static int sched_numa_scan_size_show(struct seq_file *m, void *v)
+{
+	seq_printf(m, "%u\n", sysctl_numa_balancing_scan_size);
+	return 0;
+}
+
+static int sched_numa_scan_size_open(struct inode *inode, struct file *filp)
+{
+	return single_open(filp, sched_numa_scan_size_show, NULL);
+}
+
+static const struct file_operations sched_numa_scan_size_fops = {
+	.open		= sched_numa_scan_size_open,
+	.write		= sched_numa_scan_size_write,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+#endif /* CONFIG_NUMA_BALANCING */
+
 #ifdef CONFIG_SCHED_CACHE
 static ssize_t
 sched_cache_enable_write(struct file *filp, const char __user *ubuf,
@@ -664,7 +713,7 @@ static __init int sched_init_debug(void)
 	debugfs_create_u32("scan_delay_ms", 0644, numa, &sysctl_numa_balancing_scan_delay);
 	debugfs_create_u32("scan_period_min_ms", 0644, numa, &sysctl_numa_balancing_scan_period_min);
 	debugfs_create_u32("scan_period_max_ms", 0644, numa, &sysctl_numa_balancing_scan_period_max);
-	debugfs_create_u32("scan_size_mb", 0644, numa, &sysctl_numa_balancing_scan_size);
+	debugfs_create_file("scan_size_mb", 0644, numa, NULL, &sched_numa_scan_size_fops);
 	debugfs_create_u32("hot_threshold_ms", 0644, numa, &sysctl_numa_balancing_hot_threshold);
 #endif /* CONFIG_NUMA_BALANCING */
 
-- 
2.43.0


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

* Re: [PATCH] sched/debug: Validate writes to the scan_size_mb debugfs knob
  2026-08-10  8:18 [PATCH] sched/debug: Validate writes to the scan_size_mb debugfs knob Zhan Xusheng
@ 2026-08-10 13:57 ` Chen Yu
  0 siblings, 0 replies; 2+ messages in thread
From: Chen Yu @ 2026-08-10 13:57 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Mel Gorman, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Valentin Schneider, K Prateek Nayak, zhanxusheng, linux-kernel,
	yu.c.chen

On Mon, Aug 10, 2026 at 04:18:29PM +0800, Zhan Xusheng wrote:
> +static ssize_t sched_numa_scan_size_write(struct file *filp,
> +					  const char __user *ubuf,
> +					  size_t cnt, loff_t *ppos)
> +{
> +	unsigned int mb;
> +	int ret;
> +
> +	ret = kstrtouint_from_user(ubuf, cnt, 10, &mb);
>

minor
ret = kstrtouint_from_user(ubuf, cnt, 0, &mb)?

The debugfs_create_u32() path being replaced parses with base 0 via
kstrtoull(attr->set_buf, 0, &val) in simple_attr_write_xsigned(), so
base 10 makes writes like "echo 0x100 > scan_size_mb" return -EINVAL.

Others look ok to me, with above fix, per my knowledge,

Reviewed-by: Chen Yu <yu.c.chen@intel.com>

thanks,
Chenyu 

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

end of thread, other threads:[~2026-08-10 13:58 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-10  8:18 [PATCH] sched/debug: Validate writes to the scan_size_mb debugfs knob Zhan Xusheng
2026-08-10 13:57 ` Chen Yu

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