Message-ID: <20240508113719.pccjkyd5nk5soqrg@joelS2.panther.com>
Date: Wed, 8 May 2024 13:37:19 +0200
From: Joel Granados <j.granados@...sung.com>
To: Kees Cook <keescook@...omium.org>
CC: Jakub Kicinski <kuba@...nel.org>, Luis Chamberlain <mcgrof@...nel.org>,
	Kees Cook <keescook@...omium.org>, Eric Dumazet <edumazet@...gle.com>, Dave
	Chinner <david@...morbit.com>, <linux-fsdevel@...r.kernel.org>,
	<netdev@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
	<linux-s390@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-riscv@...ts.infradead.org>, <linux-mm@...ck.org>,
	<linux-security-module@...r.kernel.org>, <bpf@...r.kernel.org>,
	<linuxppc-dev@...ts.ozlabs.org>, <linux-xfs@...r.kernel.org>,
	<linux-trace-kernel@...r.kernel.org>, <linux-perf-users@...r.kernel.org>,
	<netfilter-devel@...r.kernel.org>, <coreteam@...filter.org>,
	<kexec@...ts.infradead.org>, <linux-hardening@...r.kernel.org>,
	<bridge@...ts.linux.dev>, <lvs-devel@...r.kernel.org>,
	<linux-rdma@...r.kernel.org>, <rds-devel@....oracle.com>,
	<linux-sctp@...r.kernel.org>, <linux-nfs@...r.kernel.org>,
	<apparmor@...ts.ubuntu.com>
Subject: Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument
 of sysctl handlers

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

Download attachment "signature.asc" of type "application/pgp-signature" (660 bytes)

Powered by blists - more mailing lists