[PATCH v2] sched/debug: reject invalid writes to numa_balancing scan_size_mb

[PATCH v2] sched/debug: reject invalid writes to numa_balancing scan_size_mb

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

The sysctl_numa_balancing_scan_size parameter is currently registered
via debugfs_create_u32(), which accepts any u32 value including 0. A
zero value triggers division-by-zero in two code paths:

  task_scan_min():
    windows = MAX_SCAN_WINDOW / scan_size;

  task_nr_scan_windows():
    rss = round_up(rss, nr_scan_pages);
    return rss / nr_scan_pages;

Similarly, values exceeding UINT_MAX would be silently truncated by
the write path before reaching this handler, potentially accepting
unintended values. Reject them explicitly with -ERANGE

Replace the debugfs_create_u32() interface with a custom file_operations
handler that validates writes: reject 0 and values exceeding UINT_MAX
with -ERANGE. Since the ops use DEFINE_DEBUGFS_ATTRIBUTE() which already
provides removal protection via debugfs_file_get()/put(),
debugfs_create_file_unsafe() is used instead of debugfs_create_file() to
avoid an unnecessary full_proxy layer.

Fixes: 8a99b6833c88 ("sched: Move SCHED_DEBUG sysctl to debugfs")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
Diff with v1: replace debugfs_create_file with debugfs_create_file_unsafe;
			 and rewrite commit message

 kernel/sched/debug.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 40584b2..391a669 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -633,6 +633,26 @@ static void debugfs_fair_server_init(void)
 	}
 }
 
+#ifdef CONFIG_NUMA_BALANCING
+static int numa_scan_size_get(void *data, u64 *val)
+{
+	*val = *(u32 *)data;
+	return 0;
+}
+
+static int numa_scan_size_set(void *data, u64 val)
+{
+	if (val == 0 || val > UINT_MAX)
+		return -ERANGE;
+
+	*(u32 *)data = (u32)val;
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(numa_scan_size_fops, numa_scan_size_get,
+			 numa_scan_size_set, "%llu\n");
+#endif /* CONFIG_NUMA_BALANCING */
+
 static __init int sched_init_debug(void)
 {
 	struct dentry __maybe_unused *numa, *llc;
@@ -664,7 +684,8 @@ 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_unsafe("scan_size_mb", 0644, numa,
+			    &sysctl_numa_balancing_scan_size, &numa_scan_size_fops);
 	debugfs_create_u32("hot_threshold_ms", 0644, numa, &sysctl_numa_balancing_hot_threshold);
 #endif /* CONFIG_NUMA_BALANCING */
 
-- 
2.9.4

答复: [PATCH v2] sched/debug: reject invalid writes to numa_balancing scan_size_mb

[Li,Rongqing]

> 
> From: Li RongQing <lirongqing@baidu.com>
> 
> The sysctl_numa_balancing_scan_size parameter is currently registered via
> debugfs_create_u32(), which accepts any u32 value including 0. A zero value
> triggers division-by-zero in two code paths:
> 
>   task_scan_min():
>     windows = MAX_SCAN_WINDOW / scan_size;
> 
>   task_nr_scan_windows():
>     rss = round_up(rss, nr_scan_pages);
>     return rss / nr_scan_pages;
> 
> Similarly, values exceeding UINT_MAX would be silently truncated by the
> write path before reaching this handler, potentially accepting unintended
> values. Reject them explicitly with -ERANGE
> 
> Replace the debugfs_create_u32() interface with a custom file_operations
> handler that validates writes: reject 0 and values exceeding UINT_MAX with
> -ERANGE. Since the ops use DEFINE_DEBUGFS_ATTRIBUTE() which already
> provides removal protection via debugfs_file_get()/put(),
> debugfs_create_file_unsafe() is used instead of debugfs_create_file() to avoid
> an unnecessary full_proxy layer.
> 
> Fixes: 8a99b6833c88 ("sched: Move SCHED_DEBUG sysctl to debugfs")
> Signed-off-by: Li RongQing <lirongqing@baidu.com>

Ping

[Li,Rongqing] 

> ---
> Diff with v1: replace debugfs_create_file with debugfs_create_file_unsafe;
> 			 and rewrite commit message
> 
>  kernel/sched/debug.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c index
> 40584b2..391a669 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -633,6 +633,26 @@ static void debugfs_fair_server_init(void)
>  	}
>  }
> 
> +#ifdef CONFIG_NUMA_BALANCING
> +static int numa_scan_size_get(void *data, u64 *val) {
> +	*val = *(u32 *)data;
> +	return 0;
> +}
> +
> +static int numa_scan_size_set(void *data, u64 val) {
> +	if (val == 0 || val > UINT_MAX)
> +		return -ERANGE;
> +
> +	*(u32 *)data = (u32)val;
> +	return 0;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(numa_scan_size_fops, numa_scan_size_get,
> +			 numa_scan_size_set, "%llu\n");
> +#endif /* CONFIG_NUMA_BALANCING */
> +
>  static __init int sched_init_debug(void)  {
>  	struct dentry __maybe_unused *numa, *llc; @@ -664,7 +684,8 @@
> 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_unsafe("scan_size_mb", 0644, numa,
> +			    &sysctl_numa_balancing_scan_size,
> &numa_scan_size_fops);
>  	debugfs_create_u32("hot_threshold_ms", 0644, numa,
> &sysctl_numa_balancing_hot_threshold);
>  #endif /* CONFIG_NUMA_BALANCING */
> 
> --
> 2.9.4