Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Date:   Fri, 8 Dec 2023 10:59:26 +0100
From:   Joel Granados <j.granados@samsung.com>
To:     Thomas =?utf-8?Q?Wei=C3=9Fschuh?= <linux@weissschuh.net>
CC:     Kees Cook <keescook@chromium.org>,
        "Gustavo A. R. Silva" <gustavoars@kernel.org>,
        Luis Chamberlain <mcgrof@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: <20231208095926.aavsjrtqbb5rygmb@localhost>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg="pgp-sha512";
        protocol="application/pgp-signature"; boundary="tkudj6zge43ee23g"
Content-Disposition: inline
In-Reply-To: <fa911908-a14d-4746-a58e-caa7e1d4b8d4@t-8ch.de>
CMS-TYPE: 201P
References: <CGME20231204075237eucas1p27966f7e7da014b5992d3eef89a8fde25@eucas1p2.samsung.com>
        <20231204-const-sysctl-v2-0-7a5060b11447@weissschuh.net>
        <20231207104357.kndqvzkhxqkwkkjo@localhost>
        <fa911908-a14d-4746-a58e-caa7e1d4b8d4@t-8ch.de>
Precedence: bulk

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

On Thu, Dec 07, 2023 at 08:19:43PM +0100, Thomas Wei=DFschuh wrote:
> On 2023-12-07 11:43:57+0100, Joel Granados wrote:
> > Hey Thomas
> >=20
> > You have a couple of test bot issues for your 12/18 patch. Can you
> > please address those for your next version.
>=20
> I have these fixed locally, I assumed Luis would also pick them up
> directly until I have a proper v2, properly should have communicated
> that.
>=20
> > On Mon, Dec 04, 2023 at 08:52:13AM +0100, Thomas Wei=DFschuh wrote:
> > > Problem description:
> > >=20
> > > The kernel contains a lot of struct ctl_table throught the tree.
> > > These are very often 'static' definitions.
> > > It would be good to make the tables unmodifiable by marking them "con=
st"
> > Here I would remove "It would be good to". Just state it: "Make the
> > tables unmodifiable...."
>=20
> Ack.
>=20
> >=20
> > > to avoid accidental or malicious modifications.
> > > This is in line with a general effort to move as much data as possible
> > > into .rodata. (See for example[0] and [1])
>=20
> > If you could find more examples, it would make a better case.
>=20
> I'll look for some. So far my constifications went in without them :-)
>=20
> > >=20
> > > Unfortunately the tables can not be made const right now because the
> > > core registration functions expect mutable tables.
> > >=20
> > > This is for two main reasons:
> > >=20
> > > 1) sysctl_{set,clear}_perm_empty_ctl_header in the sysctl core modify
> > >    the table.
> > > 2) The table is passed to the handler function as a non-const pointer.
> > >=20
> > > This series migrates the core and all handlers.
>=20
> > awesome!
> >=20
> > >=20
> > > Structure of the series:
> > >=20
> > > Patch 1-3:   Cleanup patches
> > > Patch 4-7:   Non-logic preparation patches
> > > Patch 8:     Preparation patch changing a bit of logic
> > > Patch 9-12:  Treewide changes to handler function signature
> > > Patch 13-14: Adaption of the sysctl core implementation
> > > Patch 15:    Adaption of the sysctl core interface
> > > Patch 16:    New entry for checkpatch
> > > Patch 17-18: Constification of existing "struct ctl_table"s
> > >=20
> > > Tested by booting and with the sysctl selftests on x86.
> > >=20
> > > Note:
> > >=20
> > > This is intentionally sent only to a small number of people as I'd li=
ke
> > > to get some more sysctl core-maintainer feedback before sending this =
to
> > > essentially everybody.
>=20
> > When you do send it to the broader audience, you should chunk up your b=
ig
> > patches (12/18 and 11/18) and this is why:
> > 1. To avoid mail rejections from lists:
> >    You have to tell a lot of people about the changes in one mail. That
> >    will make mail header too big for some lists and it will be rejected.
> >    This happened to me with [3]
> > 2. Avoid being rejected for the wrong reasons :)
> >    Maintainers are busy ppl and sending them a set with so many files
> >    may elicit a rejection on the grounds that it involves too many
> >    subsystems at the same time.
> > I suggest you chunk it up with directories in mind. Something similar to
> > what I did for [4] where I divided stuff that when for fs/*, kernel/*,
> > net/*, arch/* and drivers/*. That will complicate your patch a tad
> > because you have to ensure that the tree can be compiled/run for every
> > commit. But it will pay off once you push it to the broader public.
>=20
> This will break bisections. All function signatures need to be switched
I was suggesting a solution without breaking bisections of course. I can
think of a couple of ways to do this in chunks but it might be
premature. You can send it and if you get push back because of this then
we can deal with chunking it down.

I'm still concerned about the header size for those mails. How does the
mail look like when you run the get maintainers script on it?

> in one step. I would strongly like to avoid introducing broken commits.
>=20
> The fact that these big commits have no functional changes at all makes
> me hope it can work.

--=20

Joel Granados

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

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

iQGzBAABCgAdFiEErkcJVyXmMSXOyyeQupfNUreWQU8FAmVy6PQACgkQupfNUreW
QU/kqwwAl4AzqtXuQrIiZVZuxbjC/B9vxdgnwDjJnHVbQ9QuNz4y3ZJq60tFYYib
uwYwA72xqqXdZFiWG5RXuqFrqMxZRVxJUKhm4NfEDW+ol0cP+fXMFE4t806KCEQF
cu+JQVXYWVhmscjWZE6zm79A1xtMLUa9wbaVnzyp6SFr0Q0B8ReM/7/xYIgHi72s
qj5rd3sj5Ss3t5l5iskdEZ4ncDU75M1fIehCRDlf5bQZSbmifuTpA5q72qTiux5W
83w3d9rTjo5f4Y9YZMG8dn06MG0CHWSBK919IS+RaiBZ9dWimNRxLgOxlstHxfMX
Dmm8zUQNl7RMPJEAkYxNoPRmZMSDR2a2mYILLTwCc/w7vUsselS8ngK+L4JdKbbA
Gl2aet6SwpjP3c/32vq+pCM9iFm6Hh4JYto4Bcqfncsa5bIvOnyiLDBvNe1hoQ0q
2vV0aY5/IOvW61UnR4p+AkQBFWPs6fln4vM2l7wIzC8pcsdEoqdmdaiyN3OdVKxF
OCQuggkL
=xR5S
-----END PGP SIGNATURE-----

--tkudj6zge43ee23g--