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

[Joel Granados]

On Sun, Dec 17, 2023 at 11:10:15PM +0100, Thomas Weißschuh wrote:
> On 2023-12-17 13:02:01+0100, Joel Granados wrote:
> > Catching up with mail....
> > 
> > On Tue, Dec 12, 2023 at 11:51:30PM -0800, Luis Chamberlain wrote:
> > > On Tue, Dec 12, 2023 at 10:09:30AM +0100, Joel Granados wrote:
> > > > My idea was to do something similar to your originl RFC, where you have
> > > > an temporary proc_handler something like proc_hdlr_const (we would need
> > > > to work on the name) and move each subsystem to the new handler while
> > > > the others stay with the non-const one. At the end, the old proc_handler
> > > > function name would disapear and would be completely replaced by the new
> > > > proc_hdlr_const.
> > > >
> > > > This is of course extra work and might not be worth it if you don't get
> > > > negative feedback related to tree-wide changes. Therefore I stick to my
> > > > previous suggestion. Send the big tree-wide patches and only explore
> > > > this option if someone screams.
> > >
> > > I think we can do better, can't we just increase confidence in that we
> > > don't *need* muttable ctl_cables with something like smatch or
> > > coccinelle so that we can just make them const?
> > >
> > > Seems like a noble endeavor for us to generalize.
> > >
> > > Then we just breeze through by first fixing those that *are* using
> > > mutable tables by having it just de-register and then re-register
> > So let me see if I understand your {de,re}-register idea:
> > When we have a situation (like in the networking code) where a ctl_table
> > is being used in an unmuttable way, we do your {de,re}-register trick.
> 
> unmuttable?
meant muttable here.

> 
> > The trick consists in unregistering an old ctl_table and reregistering
> > with a whole new const changed table. In this way, whatever we register
> > is always const.
> > 
> > Once we address all the places where this happens, then we just change
> > the handler to const and we are done.
> > 
> > Is that correct?
> 
> I'm confused.
> 
> The handlers can already be made const as shown in this series, which
> does convert the whole kernel tree.
> There is only one handler (the stackleak one) which modifies the table
> and this one is fixed as part of the series.
> 
> (Plus the changes needed to the sysctl core to avoid mutation there)
> 
> > 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.
> 
> But Luis' message was not completely clear to me.
> I guess I'm missing something.
> 
> > Can't think of anything else off the top of my head. Would have to
> > actually see the code to evaluate further I think.
> > 
> > > new tables if they need to be changed, and then a new series is sent
> > > once we fix all those muttable tables.
> 
> Thomas

-- 

Joel Granados

Attachment: signature.asc
Description: PGP signature