Re: [PATCH v5] sysctl: simplify the min/max boundary check

[Wen Yang <wen.yang@linux.dev>]

On 2025/3/10 22:25, Joel Granados wrote:
> On Thu, Mar 06, 2025 at 09:33:39PM +0800, Wen Yang wrote:
>>
>>
>> On 2025/3/3 17:26, Joel Granados wrote:
>>> On Thu, Jan 30, 2025 at 10:32:14PM +0800, Wen Yang wrote:
>>>>
>>>>
>>>> On 2025/1/28 01:51, Eric W. Biederman wrote:
>>>>> Joel Granados <joel.granados@kernel.org> writes:
>>>>>
>>>>>> On Thu, Jan 23, 2025 at 12:30:25PM -0600, Eric W. Biederman wrote:
>>>>>>> "Eric W. Biederman" <ebiederm@xmission.com> writes:
>>>>>>>
>>>>>>>> Joel Granados <joel.granados@kernel.org> writes:
>>>>>>>>
>>>>>>>>> On Sun, Jan 19, 2025 at 10:59:21PM +0800, Wen Yang wrote:
>>> ...
> ...
>>> Quick question: Do you have a systemic way of identifying these? Do you
>>> have a grep or awk scripts somewhere? I'm actually very interested in
>>> finding out what is the impact of this.
>>>
>>
>> Thanks, we may use the following simple scripts:
>>
>> - the extra {1,2} as pointers to some objects:
>> $ grep "\.extra1\|\.extra2" * -R | grep -v "SYSCTL_" | grep -v "\&"
> This is actually pretty nice. Thx for that. I executed it a bit
> differently:
> 
> $  git grep "\.extra1\|\.extra2" | grep -v "SYSCTL_" | grep -v "&"
> 
> I also went and did something way more complicated :). I created an
> smatch check [1] and ran it on a allyes config. This gave me all of your
> results except the openat2 selftests. This might be something to
> consider for when this is finished to add a check so that ppl don't just
> add an int or a long to a extra
> 
> Best
> 
> [1]: https://github.com/Joelgranados/smatch/tree/jag/extra_ptr
> 

Thanks.
This is an excellent approach!
We will also learn to apply it in our code.

--
Best wishes,
Wen

> 
>>
>> - the extra {1,2} as pointers to elements in the shared constant array:
>> $ grep "\.extra1\|\.extra2" * -R | grep "SYSCTL_"
>>
>> - the extra {1,2} as pointers to additional constant variables:
>> $ grep "\.extra1\|\.extra2" * -R | grep "\&"
>>
>>
>> --
>> Best wishes,
>> Wen
>>
>>
>>>
>>>
>>>>
>>>>
>>>> So could we modify it in this way to make it compatible with these two
>>>> situations:
>>>>
>>>> @@ -137,8 +137,16 @@ struct ctl_table {
>>>>           umode_t mode;
>>>>           proc_handler *proc_handler;     /* Callback for text formatting */
>>>>           struct ctl_table_poll *poll;
>>>> -       void *extra1;
>>>> -       void *extra2;
>>>> +       union {
>>>> +               struct {
>>>> +                       void *extra1;
>>>> +                       void *extra2;
>>>> +               };
>>>> +               struct {
>>>> +                       unsigned long min;
>>>> +                       unsigned long max;
>>>> +               };
>>>> +       };
>>>>    } __randomize_layout;
>>>>
>>>>
>>>> --
>>>> Best wishes,
>>>> Wen
>>>>
>>>
>