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: <28c3d8b8-201e-45b9-b7eb-4da3e0acf7cc@t-8ch.de>
Date: Wed, 24 Jul 2024 20:43:08 +0200
From: Thomas Weißschuh <linux@...ssschuh.net>
To: Joel Granados <j.granados@...sung.com>
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 2024-07-24 20:20:58+0000, Joel Granados wrote:
> 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.

Ok.

I ran the scripts and for the result:

Signed-off-by: Thomas Weißschuh <linux@...ssschuh.net>

> 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.

Ack.

> 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