Re: [PATCH v2 00/18] sysctl: constify sysctl ctl_tables

From: Luis Chamberlain
Date: Mon Dec 18 2023 - 16:22:16 EST


So we can split this up concentually in two:

* constificaiton of the table handlers
* constification of the table struct itself

On Sun, Dec 17, 2023 at 11:10:15PM +0100, Thomas Weißschuh wrote:
> The handlers can already be made const as shown in this series,

The series did already produce issues with some builds, and so
Julia's point is confirmed that the series only proves hanlders
which you did build and for which 0-day has coverage for.

The challenge here was to see if we could draw up a test case
that would prove this without build tests, and what occurred to
me was coccinelle or smatch.

> > If that is indeed what you are proposing, you might not even need the
> > un-register step as all the mutability that I have seen occurs before
> > the register. So maybe instead of re-registering it, you can so a copy
> > (of the changed ctl_table) to a const pointer and then pass that along
> > to the register function.
>
> Tables that are modified, but *not* through the handler, would crop
> during the constification of the table structs.
> Which should be a second step.

Instead of "croping up" at build time again, I wonder if we can do
better with coccinelle / smatch.

Joel, and yes, what you described is what I was suggesting, that is to
avoid having to add a non-const handler a first step, instead we modify
those callers which do require to modify the table by first a
deregistration and later a registration. In fact to make this even
easier a new call would be nice so to aslo be able to git grep when
this is done in the kernel.

But if what you suggest is true that there are no registrations which
later modify the table, we don't need that. It is the uncertainty that
we might have that this is a true statment that I wanted to challenge
to see if we could do better. Can we avoid this being a stupid
regression later by doing code analysis with coccinelle / smatch?

The template of the above endeavor seems useful not only to this use
case but to any place in the kernel where this previously has been done
before, and hence my suggestion that this seems like a sensible thing
to think over to see if we could generalize.

Luis