linux-kernel - Re: [RFC PATCH 1/4] sysctl: add SYSCTL_ENTRY/SYSCTL_RANGE_ENTRY helpers for ctl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <88f18992-8408-4c4c-aa57-07025256b466@linux.dev>
Date: Mon, 26 Jan 2026 04:08:46 +0800
From: Wen Yang <wen.yang@...ux.dev>
To: Joel Granados <joel.granados@...nel.org>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/4] sysctl: add SYSCTL_ENTRY/SYSCTL_RANGE_ENTRY
 helpers for ctl_table init



On 1/22/26 17:56, Joel Granados wrote:
> On Wed, Jan 14, 2026 at 01:40:30AM +0800, wen.yang@...ux.dev wrote:
>> From: Wen Yang <wen.yang@...ux.dev>
>>
>> Introduce SYSCTL_ENTRY() and SYSCTL_RANGE_ENTRY() (built on __SYSCTL_ENTRY())
>> to consolidate common struct ctl_table initializers in one place.
>> This is a preparatory refactor in support of future tree-wide sysctl
>> cleanups by making the how to build a ctl_table entry logic uniform.
>>
>> Also add SYSCTL_NULL for explicitly passing a NULL .data pointer through
>> the new macros, and convert a few in-tree users (kernel/sysctl.c) to
>> validate the new helpers.
>>
>> No functional change intended.
>>
>> Selftest were also made:
>>
>> [19:31:40] Starting KUnit Kernel (1/1)...
>> [19:31:40] ============================================================
>> Running tests with:
>> $ .kunit/linux kunit.filter_glob=sysctl_test kunit.enable=1 mem=1G console=tty kunit_shutdown=halt
>> [19:31:40] ================ sysctl_test (10 subtests) =================
>> [19:31:40] [PASSED] sysctl_test_api_dointvec_null_tbl_data
>> [19:31:40] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset
>> [19:31:40] [PASSED] sysctl_test_api_dointvec_table_len_is_zero
>> [19:31:40] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set
>> [19:31:40] [PASSED] sysctl_test_dointvec_read_happy_single_positive
>> [19:31:40] [PASSED] sysctl_test_dointvec_read_happy_single_negative
>> [19:31:40] [PASSED] sysctl_test_dointvec_write_happy_single_positive
>> [19:31:40] [PASSED] sysctl_test_dointvec_write_happy_single_negative
>> [19:31:40] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min
>> [19:31:40] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max
>> [19:31:40] =================== [PASSED] sysctl_test ===================
>> [19:31:40] ============================================================
>> [19:31:40] Testing complete. Ran 10 tests: passed: 10
>> [19:31:40] Elapsed time: 26.755s total, 0.002s configuring, 26.632s building, 0.102s running
>>
>> Suggested-by: Joel Granados <joel.granados@...nel.org>
>> Link: https://lore.kernel.org/all/7mevcgca7xsd5gckrap22byjinhrgqnelu4y7hzo2r5t2cq7w4@nyaa42khwqez/#t
>> Signed-off-by: Wen Yang <wen.yang@...ux.dev>
>> ---
>>   include/linux/sysctl.h |  18 ++++++
>>   kernel/sysctl-test.c   | 131 ++++++++++-------------------------------
>>   kernel/sysctl.c        |  43 ++------------
>>   3 files changed, 56 insertions(+), 136 deletions(-)
>>
>> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
>> index 2886fbceb5d6..683422bc3e7f 100644
>> --- a/include/linux/sysctl.h
>> +++ b/include/linux/sysctl.h
>> @@ -53,6 +53,7 @@ struct ctl_dir;
>>   #define SYSCTL_MAXOLDUID		((void *)&sysctl_vals[10])
>>   #define SYSCTL_NEG_ONE			((void *)&sysctl_vals[11])
>>   
>> +#define SYSCTL_NULL			((void *)NULL)
>>   extern const int sysctl_vals[];
>>   
>>   #define SYSCTL_LONG_ZERO	((void *)&sysctl_long_vals[0])
>> @@ -175,6 +176,23 @@ struct ctl_table {
>>   	void *extra2;
>>   } __randomize_layout;
>>   
>> +#define __SYSCTL_ENTRY(NAME, DATA, TYPE, MODE, HANDLER, SMIN, SMAX)\
>> +	{							\
>> +		.procname	= NAME,				\
>> +		.data		= DATA,				\
> I would like to see at least one lvalue check for the macro parameters.
> Something like this
> 
> .data = (*(typeof(DATA)*)&(DATA))
> 
> In such a way that the compiler fails if an expression is passed.
> We can handle these cases (if they exist) in some other way
> 
>> +		.maxlen		= sizeof(TYPE),			\
>> +		.mode		= MODE,				\
>> +		.proc_handler	= HANDLER,			\
>> +		.extra1		= SMIN,				\
>> +		.extra2		= SMAX,				\
>> +	}
>> +
>> +#define SYSCTL_ENTRY(NAME, DATA, TYPE, MODE)			\
>> +	__SYSCTL_ENTRY(NAME, DATA, TYPE, MODE, proc_do##TYPE##vec, NULL, NULL)
>> +
>> +#define SYSCTL_RANGE_ENTRY(NAME, DATA, TYPE, MODE, SMIN, SMAX)	\
> I'm leaning more towards SYSCTL_ENTRY_RANGE or SYSCTL_ENTRY_R.
> 
> I am also keeping notes here if you are interested
> https://sysctl-dev-rtd.readthedocs.io/en/latest/notes/ctltable_entry_macro.html#table-entry-macro
> 

Great, thank you for your detailed comments.
These suggestions are indeed the best, implementable, robust, and highly 
readable.
I will soon make these changes according to them soon, probably within a 
day or two.


--
Best wishes,
Wen

> 
> 
>> +	__SYSCTL_ENTRY(NAME, DATA, TYPE, MODE, proc_do##TYPE##vec_minmax, SMIN, SMAX)
>> +
>>   struct ctl_node {
>>   	struct rb_node node;
>>   	struct ctl_table_header *header;
>> diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
>> index 92f94ea28957..7fb0e7f1e62f 100644
>> --- a/kernel/sysctl-test.c
>> +++ b/kernel/sysctl-test.c
>> @@ -15,20 +15,14 @@
> ....
>> -	{
>> -		.procname	= "ignore-unaligned-usertrap",
>> -		.data		= &no_unaligned_warning,
>> -		.maxlen		= sizeof (int),
>> -		.mode		= 0644,
>> -		.proc_handler	= proc_dointvec,
>> -	},
>> +	SYSCTL_ENTRY("ignore-unaligned-usertrap", &no_unaligned_warning, int, 0644),
>>   #endif
>>   };
>>   
>> -- 
>> 2.25.1
>>
>