From b5ffbd1396885f76bf87e67d590a3ef063e6d831 Mon Sep 17 00:00:00 2001 From: Wen Yang Date: Fri, 19 Apr 2024 11:36:39 +0800 Subject: sysctl: move the extra1/2 boundary check of u8 to sysctl_check_table_array Move boundary checking for proc_dou8ved_minmax into module loading, thereby reporting errors in advance. And add a kunit test case ensuring the boundary check is done correctly. The boundary check in proc_dou8vec_minmax done to the extra elements in the ctl_table struct is currently performed at runtime. This allows buggy kernel modules to be loaded normally without any errors only to fail when used. This is a buggy example module: #include #include #include static struct ctl_table_header *_table_header = NULL; static unsigned char _data = 0; struct ctl_table table[] = { { .procname = "foo", .data = &_data, .maxlen = sizeof(u8), .mode = 0644, .proc_handler = proc_dou8vec_minmax, .extra1 = SYSCTL_ZERO, .extra2 = SYSCTL_ONE_THOUSAND, }, }; static int init_demo(void) { _table_header = register_sysctl("kernel", table); if (!_table_header) return -ENOMEM; return 0; } module_init(init_demo); MODULE_LICENSE("GPL"); And this is the result: # insmod test.ko # cat /proc/sys/kernel/foo cat: /proc/sys/kernel/foo: Invalid argument Suggested-by: Joel Granados Signed-off-by: Wen Yang Cc: Luis Chamberlain Cc: Kees Cook Cc: Joel Granados Cc: Eric W. Biederman Cc: Christian Brauner Cc: linux-kernel@vger.kernel.org Reviewed-by: Joel Granados Signed-off-by: Joel Granados --- kernel/sysctl.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'kernel/sysctl.c') diff --git a/kernel/sysctl.c b/kernel/sysctl.c index e0b917328cf9..c0a1164eaf59 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -977,16 +977,10 @@ int proc_dou8vec_minmax(struct ctl_table *table, int write, if (table->maxlen != sizeof(u8)) return -EINVAL; - if (table->extra1) { + if (table->extra1) min = *(unsigned int *) table->extra1; - if (min > 255U) - return -EINVAL; - } - if (table->extra2) { + if (table->extra2) max = *(unsigned int *) table->extra2; - if (max > 255U) - return -EINVAL; - } tmp = *table; -- cgit v1.2.3