Received-SPF: pass (google.com: domain of linux-nfs+bounces-3201-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Wed, 8 May 2024 13:37:19 +0200
From: Joel Granados <j.granados@samsung.com>
To: Kees Cook <keescook@chromium.org>
CC: Jakub Kicinski <kuba@kernel.org>, Luis Chamberlain <mcgrof@kernel.org>,
	Kees Cook <keescook@chromium.org>, Eric Dumazet <edumazet@google.com>, Dave
	Chinner <david@fromorbit.com>, <linux-fsdevel@vger.kernel.org>,
	<netdev@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>,
	<linux-s390@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-riscv@lists.infradead.org>, <linux-mm@kvack.org>,
	<linux-security-module@vger.kernel.org>, <bpf@vger.kernel.org>,
	<linuxppc-dev@lists.ozlabs.org>, <linux-xfs@vger.kernel.org>,
	<linux-trace-kernel@vger.kernel.org>, <linux-perf-users@vger.kernel.org>,
	<netfilter-devel@vger.kernel.org>, <coreteam@netfilter.org>,
	<kexec@lists.infradead.org>, <linux-hardening@vger.kernel.org>,
	<bridge@lists.linux.dev>, <lvs-devel@vger.kernel.org>,
	<linux-rdma@vger.kernel.org>, <rds-devel@oss.oracle.com>,
	<linux-sctp@vger.kernel.org>, <linux-nfs@vger.kernel.org>,
	<apparmor@lists.ubuntu.com>
Subject: Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument
 of sysctl handlers
Message-ID: <20240508113719.pccjkyd5nk5soqrg@joelS2.panther.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg="pgp-sha512";
	protocol="application/pgp-signature"; boundary="v7sa5w52x2sbyktg"
Content-Disposition: inline
In-Reply-To: <d11f875e-4fb5-46dd-a412-84818208c575@t-8ch.de>
CMS-TYPE: 201P
References: <20240423-sysctl-const-handler-v3-0-e0beccb836e2@weissschuh.net>
	<CGME20240425031241eucas1p1fb0790e0d03ccbe4fca2b5f6da83d6db@eucas1p1.samsung.com>
	<20240424201234.3cc2b509@kernel.org>
	<20240425110412.2n5d27smecfncsfa@joelS2.panther.com>
	<d11f875e-4fb5-46dd-a412-84818208c575@t-8ch.de>

--v7sa5w52x2sbyktg
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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].
> >=20
> > Here are some comments about the alternative:
> > 1. We would need to make the first argument const in all the derived
> >    proc_handlers [3]=20
> > 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.
> >=20
> > I would really like to hear what ppl think about this "heretic"
> > alternative. @Thomas, @Luis, @Kees @Jakub?
>=20
> Thanks for that alternative, I'm not a big fan though.
>=20
> 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.
>=20
> 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.
>=20
> But if the consensus prefers this solution, I'll be happy to adopt it.
>=20
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux=
=2Egit/commit/?h=3Djag/constfy_treewide_alternative&id=3D4a383503b1ea650d4e=
12c1f5838974e879f5aa6f
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux=
=2Egit/commit/?h=3Djag/constfy_treewide_alternative&id=3Da3be65973d27ec2933=
b9e81e1bec60be3a9b460d
> > [3] proc_dostring, proc_dobool, proc_dointvec....
>=20
>=20
> Thomas

Best
--=20

Joel Granados

--v7sa5w52x2sbyktg
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQGzBAABCgAdFiEErkcJVyXmMSXOyyeQupfNUreWQU8FAmY7Y+4ACgkQupfNUreW
QU9YEgwAhDVP7Y6eoR6THZeBhX5jlhu2G9nJstttqNF2XutpFKVjE8Z3C89eHNng
DCMo7QPRNspNV7452TWHzjB9TyWvZ3WtKaNd1O0ECwCMpZ32/0aZ5SAsMd+cY2D5
yDhAjIzVJups6lQWx1bTMDtuWft9Ebi7Wvd9I3Q+Qjq5khbrSHMCzCr1Bl2ZfscY
ApmHqb3v0KxOM+QrrybFvFCD/tt7uYyJfmH96GgBLdl2+7ceoUigiSK5ebySplCo
UnNLZi6LEwS1kjyNpbWQbKq9czcSKCCYVM3kzqzcPBdRU/km7XH0oi7SybNt7PSz
ZvFQgc34OiFAFcb9twbtzTKfXVBXpBwmOwwgqLw4yNS2C8QH+nbO6qUmHoDp44ZW
6jYrjdWbdDrP+Mwh56FkIcs84+0tgdLgsT6ni9LyK50S8Ik7ivQRCnojjpNJS19p
wRnfmB6C3lNH2MWxq9gw8k+RMhDB3eRxcXrTbEjYImsO4zb3xo+CjjPG5TGkvezg
ehHTRa51
=YpMU
-----END PGP SIGNATURE-----

--v7sa5w52x2sbyktg--