Re: [PATCH v2] sysctl: treewide: constify the ctl_table argument of proc_handlers

[Kees Cook <kees@kernel.org>]

On Wed, Jul 24, 2024 at 11:05:09PM +0200, Joel Granados wrote:
> On Wed, Jun 19, 2024 at 01:51:15PM -0700, Kees Cook wrote:
> > On Wed, Jun 19, 2024 at 12:09:00PM +0200, Thomas Wei?schuh wrote:
> > > Adapt the proc_hander function signature to make it clear that handlers
> > > are not supposed to modify their ctl_table argument.
> > > 
> > > This is also a prerequisite to moving the static ctl_table structs into
> > > read-only data.
> > > 
> > > The patch was mostly generated by coccinelle with the following script:
> > > 
> > >     @@
> > >     identifier func, ctl, write, buffer, lenp, ppos;
> > >     @@
> > > 
> > >     int func(
> > >     - struct ctl_table *ctl,
> > >     + const struct ctl_table *ctl,
> > >       int write, void *buffer, size_t *lenp, loff_t *ppos)
> > >     { ... }
> > > 
> > > In addition to the scripted changes some other changes are done:
> > > 
> > > * The "typedef proc_handler" in include/linux/sysctl.h is changed to use
> > >   the "const ctl_table".
> > > 
> > > * The prototypes of non-static handlers in header-files are adapted
> > >   to match the changes of their respective definitions.
> > > 
> > > * kernel/watchdog.c: proc_watchdog_common()
> > >   This is called from a proc_handler itself and is als calling back
> > >   into another proc_handler, making it necessary to change it as part
> > >   of the proc_handler migration.
> > > 
> > > No functional change.
> > > 
> > > Signed-off-by: Thomas Wei?schuh <linux@weissschuh.net>
> > 
> > This looks good! I'll be glad to see these all be read-only. :)
> > 
> > Reviewed-by: Kees Cook <kees@kernel.org>
> Since the patch and the coccinelle script changed, I removed your
> Reviewed-by tag from the commit that I sent. Just FYI.

Totally reasonable. And the PR looks good. We'll see what Linus thinks! :)

-- 
Kees Cook