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

[Wen Yang]

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@xxxxxxxxxx> writes:
> > > > >
> > > > > > On Thu, Jan 23, 2025 at 12:30:25PM -0600, Eric W. Biederman wrote:
> > > > > > > "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx> writes:
> > > > > > >
> > > > > > > > Joel Granados <joel.granados@xxxxxxxxxx> 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