Received-SPF: pass (google.com: domain of linux-kernel+bounces-10403-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: Thu, 21 Dec 2023 13:36:44 +0100
From: Joel Granados <j.granados@samsung.com>
To: Luis Chamberlain <mcgrof@kernel.org>
CC: Thomas =?utf-8?Q?Wei=C3=9Fschuh?= <linux@weissschuh.net>, Dan Carpenter
	<dan.carpenter@linaro.org>, Julia Lawall <julia.lawall@inria.fr>, Kees Cook
	<keescook@chromium.org>, "Gustavo A. R. Silva" <gustavoars@kernel.org>,
	Iurii Zaikin <yzaikin@google.com>, Greg Kroah-Hartman
	<gregkh@linuxfoundation.org>, <linux-hardening@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v2 00/18] sysctl: constify sysctl ctl_tables
Message-ID: <20231221123644.k57uvfmpcrfo4vfk@localhost>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg="pgp-sha512";
	protocol="application/pgp-signature"; boundary="brxurlnbrxj2ydht"
Content-Disposition: inline
In-Reply-To: <ZYC37Vco1p4vD8ji@bombadil.infradead.org>
CMS-TYPE: 201P
References: <20231204-const-sysctl-v2-0-7a5060b11447@weissschuh.net>
	<20231207104357.kndqvzkhxqkwkkjo@localhost>
	<fa911908-a14d-4746-a58e-caa7e1d4b8d4@t-8ch.de>
	<20231208095926.aavsjrtqbb5rygmb@localhost>
	<8509a36b-ac23-4fcd-b797-f8915662d5e1@t-8ch.de>
	<20231212090930.y4omk62wenxgo5by@localhost>
	<ZXligolK0ekZ+Zuf@bombadil.infradead.org>
	<20231217120201.z4gr3ksjd4ai2nlk@localhost>
	<908dc370-7cf6-4b2b-b7c9-066779bc48eb@t-8ch.de>
	<ZYC37Vco1p4vD8ji@bombadil.infradead.org>
	<CGME20231223120907eucas1p20afac63076e1e9d5aee6adaa101c0630@eucas1p2.samsung.com>

--brxurlnbrxj2ydht
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Dec 18, 2023 at 01:21:49PM -0800, Luis Chamberlain wrote:
> So we can split this up concentually in two:
>=20
>  * constificaiton of the table handlers
>  * constification of the table struct itself
>=20
> On Sun, Dec 17, 2023 at 11:10:15PM +0100, Thomas Wei=DFschuh wrote:
> > The handlers can already be made const as shown in this series,
>=20
> The series did already produce issues with some builds, and so
> Julia's point is confirmed that the series only proves hanlders
> which you did build and for which 0-day has coverage for.
>=20
> The challenge here was to see if we could draw up a test case
> that would prove this without build tests, and what occurred to
> me was coccinelle or smatch.
>=20
> > > If that is indeed what you are proposing, you might not even need the
> > > un-register step as all the mutability that I have seen occurs before
> > > the register. So maybe instead of re-registering it, you can so a copy
> > > (of the changed ctl_table) to a const pointer and then pass that along
> > > to the register function.
> >=20
> > Tables that are modified, but *not* through the handler, would crop
> > during the constification of the table structs.
> > Which should be a second step.
>=20
> Instead of "croping up" at build time again, I wonder if we can do
> better with coccinelle / smatch.
>=20
> Joel, and yes, what you described is what I was suggesting, that is to
> avoid having to add a non-const handler a first step, instead we modify
> those callers which do require to modify the table by first a
> deregistration and later a registration. In fact to make this even
> easier a new call would be nice so to aslo be able to git grep when
> this is done in the kernel.
>=20
> But if what you suggest is true that there are no registrations which
> later modify the table, we don't need that. It is the uncertainty that
> we might have that this is a true statment that I wanted to challenge
> to see if we could do better. Can we avoid this being a stupid
> regression later by doing code analysis with coccinelle / smatch?
That would be amazing! Having an analysis (coccinelle or smatch) that
prevents the regression would be the cherry on top.

So to further clarify: what you propose is an additional patch in the
series that adds this regression prevention.
Or do you want a general analysis that prevents changing a variable
variable that was defined as const.

>=20
> The template of the above endeavor seems useful not only to this use
> case but to any place in the kernel where this previously has been done
> before, and hence my suggestion that this seems like a sensible thing
> to think over to see if we could generalize.
A generalized analysis for inadvertent de-constification would be great.
It would point to places where we need remove the const qualifier or add
it.

>=20
>   Luis

--=20

Joel Granados

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

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

iQGzBAABCgAdFiEErkcJVyXmMSXOyyeQupfNUreWQU8FAmWEMU4ACgkQupfNUreW
QU+gqQv/cDd/9us9hD8vyW098KoGCIcBbMRHiVL+SKCPxwm/2uurRVr0kwyaAAXz
WczZRYjOMTCBV35W7pqe4TvsE22QmSua+NcttmkNEU1gH+Yxy/fWSp5so7FQ2EO9
Z6NMYBYwntja/Mz/sOfPBbLBn3SILLfudVKBA5AA5OjqN6hqvnKQyIvFLqXpOcPK
weKWk7iq7n7Zht/1mBqVNWG/lMONqurUPj3crYd0wbLY/FvPBCDKXX+bT12Xa2+A
FdBw40R26p8BrfK/zfMRcOCwMx9QLVGr+gN0gs7kryWuyiI+dhXvOvq7cSFskxfg
vRIuEWeMbO8JD1EWE399BlQrjzPmEtEzlBzDzTw25g3k8w89jw5YNLrU9cteGUO2
DVg2qQn4slGjTl9D3mxyFFJy1/F/pnsXK379F0qm8C5WQB0R/tmPlgxJB24mYsmL
A4ygA2gMG/gYGGkYdWx4E0r7eIod/n5JxMRuFl4n43U80fiDIhpedhSWYGWFPE9d
6zd+JFPc
=KObS
-----END PGP SIGNATURE-----

--brxurlnbrxj2ydht--