lkml.org 
[lkml]   [2024]   [Apr]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2] sysctl: move the extra1/2 boundary check of u8 to sysctl_check_table_array
From


On 2024/3/29 00:53, Joel Granados wrote:
> On Sat, Mar 02, 2024 at 11:27:45PM +0800, wenyang.linux@foxmail.com wrote:
>> From: Wen Yang <wenyang.linux@foxmail.com>
>>
>
> <--- snip --->
>
>> diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
>> index 6ef887c19c48..84e759a8328f 100644
>> --- a/kernel/sysctl-test.c
>> +++ b/kernel/sysctl-test.c
>> @@ -367,6 +367,35 @@ static void sysctl_test_api_dointvec_write_single_greater_int_max(
>> KUNIT_EXPECT_EQ(test, 0, *((int *)table.data));
>> }
>>
>> +/*
>> + * Test that registering an invalid extra value is not allowed.
>> + */
>> +static void sysctl_test_register_sysctl_sz_invalid_extra_value(
>> + struct kunit *test)
>> +{
>> + unsigned char data = 0;
>> + struct ctl_table table[] = {
> I'm pretty sure that this is going to clash with the constification that
> Thomas is working on. Please re-work this patch knowing that these
> ctl_tables are going to have to change to const at some point.
>
>> + {
>> + .procname = "foo",
>> + .data = &data,
>> + .maxlen = sizeof(u8),
>> + .mode = 0644,
>> + .proc_handler = proc_dou8vec_minmax,
>> + .extra1 = SYSCTL_ZERO,
>> + .extra2 = SYSCTL_ONE_THOUSAND,
>> + },
>> + {}
> Don't use the sentinel here. We are removing them and all new sysctl
> tables (even the test ones) should be created without them
>
>> + };
>> + unsigned int size = ARRAY_SIZE(table);
> You do not need size here. When you use register_sysctl, the size will
> be automatically calculated for you.
>
>> +
>> + KUNIT_EXPECT_NULL(test, register_sysctl_sz("foo", table, size));
>> + table[0].extra1 = SYSCTL_ONE_THOUSAND;
>> + KUNIT_EXPECT_NULL(test, register_sysctl_sz("foo", table, size));
>> + table[0].extra1 = SYSCTL_ONE_HUNDRED;
>> + table[0].extra2 = SYSCTL_TWO_HUNDRED;
>> + KUNIT_EXPECT_NOT_NULL(test, register_sysctl_sz("foo", table, size));
> Replace all these by register_sysctl please.
>
Thanks for your review comments w.r.t [PATCH v2], which will be complied
by in next patch version [PATCH v3].

--
Best wishes,
Wen

>> +}
>> +
>> static struct kunit_case sysctl_test_cases[] = {
>> KUNIT_CASE(sysctl_test_api_dointvec_null_tbl_data),
>> KUNIT_CASE(sysctl_test_api_dointvec_table_maxlen_unset),
>> @@ -378,6 +407,7 @@ static struct kunit_case sysctl_test_cases[] = {
>> KUNIT_CASE(sysctl_test_dointvec_write_happy_single_negative),
>> KUNIT_CASE(sysctl_test_api_dointvec_write_single_less_int_min),
>> KUNIT_CASE(sysctl_test_api_dointvec_write_single_greater_int_max),
>> + KUNIT_CASE(sysctl_test_register_sysctl_sz_invalid_extra_value),
>> {}
>> };
>>
>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>> index f67b39d3d6e5..28888744626a 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) {
>> - min = *(unsigned int *) table->extra1;
>> - if (min > 255U)
>> - return -EINVAL;
>> - }
>> - if (table->extra2) {
>> - max = *(unsigned int *) table->extra2;
>> - if (max > 255U)
>> - return -EINVAL;
>> - }
>> + if (table->extra1)
>> + min = *(unsigned char *) table->extra1;
>> + if (table->extra2)
>> + max = *(unsigned char *) table->extra2;
>>
>> tmp = *table;
>>
>> --
>> 2.25.1
>>
>


\
 
 \ /
  Last update: 2024-05-27 16:19    [W:1.568 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site