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

[Joel Granados]

On Mon, Dec 18, 2023 at 01:21:49PM -0800, Luis Chamberlain wrote:
> 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?
That would be amazing! Having an analysis (coccinelle or smatch) that
prevents the regression would be the cherry on top.

So to further clarify: what you propose is an additional patch in the
series that adds this regression prevention.
Or do you want a general analysis that prevents changing a variable
variable that was defined as const.

> 
> 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.
A generalized analysis for inadvertent de-constification would be great.
It would point to places where we need remove the const qualifier or add
it.

> 
>   Luis

-- 

Joel Granados

Attachment: signature.asc
Description: PGP signature