Re: Current state of the sysctl constification effort

[Joel Granados]

On Fri, Jun 07, 2024 at 03:48:20PM +0200, Thomas Weißschuh wrote:
> On 2024-06-07 11:30:53+0000, Joel Granados wrote:
> > On Thu, Jun 06, 2024 at 11:52:25AM -0700, Kees Cook wrote:
> > > On Wed, Jun 05, 2024 at 10:26:25AM +0200, Joel Granados wrote:
> > > > On Fri, May 31, 2024 at 09:31:24AM -0700, Kees Cook wrote:
...
> > > 
> > > > 2. Is there a special way to send these treewide patches? Or is it just
> > > >    a regular PR with an explanation on why it is being done?
> > > 
> > > I would do a regular PR with all the details for Linus to do the change
> > > himself, but many times people send these as an explicit patch. For
> > > example, include the full Coccinelle script, or the "sed" command
> > > line, etc, and then detail any "by hand" changes that were needed on
> > > top of that.
> > @Thomas: have you sent the 11/11 patch on its own to the lists? I cant
> > find it in my history. Please send it as a stand-alone patch, so It can
> > go into sysctl just like the others.
> 
> No, I didn't send it to the list on its own yet.
> Do you want some changes or can I send it as-is?
> (Plus the new motivational blurb)

Please work on the commit message; no need to change the diff.

Here is more specific feedback on how to change the message in [1]

1. Say what was done in the first sentence. Something similar to this:
  "Add the const qualifier to the proc_handler function signatures to
  make clear...."

2. Include the general constification motivation. Something similar to
   this:
   "This patch is a prerequisite to moving all static ctl_talbe structs
   into .rodata which will reduce the attack surface in sysctl by
   ensuring that proc_handler function pointers cannot be changed."

3. No need to mention that this is to avoid lengthy transition. Please
   remove it from the commit. You can add it to the cover letter or just
   leave it out altogether. Up to you.

4. Please leave the cocci script. But I would be more specific on the
   rest of the changes. Something like this:

   "
   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 cocci changes:
   * Added a const qualifier to the ctl_table argument of the
     proc_handler typedef.

   * ... Change the others accordingly ...

   "

Best

-- 

Joel Granados