linux-kernel - Re: [PATCH v2] sysctl: treewide: constify the ctl_table argument of proc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240724182058.svxh3ux23p4fnqqf@joelS2.panther.com>
Date: Wed, 24 Jul 2024 20:20:58 +0200
From: Joel Granados <j.granados@...sung.com>
To: Thomas Weißschuh <linux@...ssschuh.net>
CC: Luis Chamberlain <mcgrof@...nel.org>, Kees Cook <kees@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] sysctl: treewide: constify the ctl_table argument of
 proc_handlers

On Wed, Jul 24, 2024 at 11:32:26AM +0200, Thomas Weißschuh wrote:
> On 2024-07-24 11:18:17+0000, Joel Granados wrote:
...
> > > > ```
> > > 
> > > Yes, IIRC I got told during one review to drop it.
> > > But for me it is also necessary.
> > I also remember a comment from the XFS part of the patches where you
> > changed the formatting on some functions. What did you do to address
> > this? Did you change them manually? Or do you have a script?
> 
> Yes, the style fixes were done manually.
Ok. I have created some sed scripts to automate this.

> 
> > > Thinking back, there were other "virtual" lines, too.
...
> >   int , void *, size_t *, loff_t *);
> > ```
> > 
> > > 
> > > (It doesn't find anything else, though)
> > Maybe you are missing running it with --include-headers?
> 
> There seems to be a slight misunderstanding.
> I meant that it does not find anything *new* on top of the existing
> patch, not that it can fully recreate the patch.
Sorry, misunderstood you.

> 
> (I used --include-headers, undid one header hunk from the patch and
> validated that the hunk gets recreated)
ok. That means that "--include-headers" is active.

I actually made further changes on the cocci script for it to catch all
the corner cases that you did by hand. The patch changes substantially
as now there are much less formatting issues introduced by the script.
I'll include you as co-developed-by and in that way give you proper
credit for your work.

In order to reproduce what I did you need to run [1] with this cocci
script [2] and then run [3] with this sed script [4]. I have checked to
see that the same files are changed. The difference between what you and
I did is mainly that coccinelle does not make any formatting decisions.

Will create and send the PR shortly.

Best


[1]
```
  make coccicheck  MODE=patch SPFLAGS="--in-place --include-headers --smpl-spacing --jobs=8" COCCI=SCRIPT.cocci
```

[2]
```
  virtual patch

  @r1@
  identifier ctl, write, buffer, lenp, ppos;
  identifier func !~ "appldata_(timer|interval)_handler|sched_(rt|rr)_handler|rds_tcp_skbuf_handler|proc_sctp_do_(hmac_alg|rto_min|rto_max|udp_port|alpha_beta|auth|probe_interval)";
  @@

  int func(
  - struct ctl_table *ctl
  + const struct ctl_table *ctl
    ,int write, void *buffer, size_t *lenp, loff_t *ppos);

  @r2@
  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)
  { ... }

  @r3@
  identifier func;
  @@

  int func(
  - struct ctl_table *
  + const struct ctl_table *
    ,int , void *, size_t *, loff_t *);

  @r4@
  identifier func, ctl;
  @@

  int func(
  - struct ctl_table *ctl
  + const struct ctl_table *ctl
    ,int , void *, size_t *, loff_t *);

  @r5@
  identifier func, write, buffer, lenp, ppos;
  @@

  int func(
  - struct ctl_table *
  + const struct ctl_table *
    ,int write, void *buffer, size_t *lenp, loff_t *ppos);
```

[3] 
```
 sed --in-place -f /tmp/constfy_formating.sed fs/xfs/xfs_sysctl.c kernel/watchdog.c
```

[4]
```
s/^xfs_stats_clear_proc_handler(const struct ctl_table \*ctl,$/xfs_stats_clear_proc_handler(\
\tconst struct ctl_table\t*ctl,/
s/^xfs_panic_mask_proc_handler(const struct ctl_table \*ctl,$/xfs_panic_mask_proc_handler(\
\tconst struct ctl_table\t*ctl,/
s/^xfs_deprecated_dointvec_minmax(const struct ctl_table \*ctl,$/xfs_deprecated_dointvec_minmax(\
\tconst struct ctl_table\t*ctl,/
s/proc_watchdog_common(int which, struct ctl_table \*table/proc_watchdog_common(int which, const struct ctl_table *table/

```

-- 

Joel Granados