LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sy

To: Kees Cook <keescook@xxxxxxxxxxxx>
Subject: Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers
Cc: Jakub Kicinski <kuba@xxxxxxxxxx>, Luis Chamberlain <mcgrof@xxxxxxxxxx>, Kees Cook <keescook@xxxxxxxxxxxx>, Eric Dumazet <edumazet@xxxxxxxxxx>, Dave Chinner <david@xxxxxxxxxxxxx>, <linux-fsdevel@xxxxxxxxxxxxxxx>, <netdev@xxxxxxxxxxxxxxx>, <linux-arm-kernel@xxxxxxxxxxxxxxxxxxx>, <linux-s390@xxxxxxxxxxxxxxx>, <linux-kernel@xxxxxxxxxxxxxxx>, <linux-riscv@xxxxxxxxxxxxxxxxxxx>, <linux-mm@xxxxxxxxx>, <linux-security-module@xxxxxxxxxxxxxxx>, <bpf@xxxxxxxxxxxxxxx>, <linuxppc-dev@xxxxxxxxxxxxxxxx>, <linux-xfs@xxxxxxxxxxxxxxx>, <linux-trace-kernel@xxxxxxxxxxxxxxx>, <linux-perf-users@xxxxxxxxxxxxxxx>, <netfilter-devel@xxxxxxxxxxxxxxx>, <coreteam@xxxxxxxxxxxxx>, <kexec@xxxxxxxxxxxxxxxxxxx>, <linux-hardening@xxxxxxxxxxxxxxx>, <bridge@xxxxxxxxxxxxxxx>, <lvs-devel@xxxxxxxxxxxxxxx>, <linux-rdma@xxxxxxxxxxxxxxx>, <rds-devel@xxxxxxxxxxxxxx>, <linux-sctp@xxxxxxxxxxxxxxx>, <linux-nfs@xxxxxxxxxxxxxxx>, <apparmor@xxxxxxxxxxxxxxxx>
From: Joel Granados <j.granados@xxxxxxxxxxx>
Date: Wed, 8 May 2024 13:37:19 +0200
Kees

Could you comment on the feasibility of this alternative from the
Control Flow Integrity perspective. My proposal is to change the
proc_handler to void* and back in the same release. So there would not
be a kernel released with a void* proc_handler.

> > However, there is an alternative way to do this that allows chunking. We
> > first define the proc_handler as a void pointer (casting it where it is
> > being used) [1]. Then we could do the constification by subsystem (like
> > Jakub proposes). Finally we can "revert the void pointer change so we
> > don't have one size fit all pointer as our proc_handler [2].
> > 
> > Here are some comments about the alternative:
> > 1. We would need to make the first argument const in all the derived
> >    proc_handlers [3] 
> > 2. There would be no undefined behavior for two reasons:
> >    2.1. There is no case where we change the first argument. We know
> >         this because there are no compile errors after we make it const.
> >    2.2. We would always go from non-const to const. This is the case
> >         because all the stuff that is unchanged in non-const.
> > 3. If the idea sticks, it should go into mainline as one patchset. I
> >    would not like to have a void* proc_handler in a kernel release.
> > 4. I think this is a "win/win" solution were the constification goes
> >    through and it is divided in such a way that it is reviewable.
> > 
> > I would really like to hear what ppl think about this "heretic"
> > alternative. @Thomas, @Luis, @Kees @Jakub?
> 
> Thanks for that alternative, I'm not a big fan though.
> 
> Besides the wonky syntax, Control Flow Integrity should trap on
> this construct. Functions are called through different pointers than
> their actual types which is exactly what CFI is meant to prevent.
> 
> Maybe people find it easier to review when using
> "--word-diff" and/or "-U0" with git diff/show.
> There is really nothing going an besides adding a few "const"s.
> 
> But if the consensus prefers this solution, I'll be happy to adopt it.
> 
> > [1] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux.git/commit/?h=jag/constfy_treewide_alternative&id=4a383503b1ea650d4e12c1f5838974e879f5aa6f
> > [2] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux.git/commit/?h=jag/constfy_treewide_alternative&id=a3be65973d27ec2933b9e81e1bec60be3a9b460d
> > [3] proc_dostring, proc_dobool, proc_dointvec....
> 
> 
> Thomas

Best
-- 

Joel Granados

Attachment: signature.asc
Description: PGP signature

<Prev in Thread] Current Thread [Next in Thread>