Received-SPF: pass (google.com: domain of linux-kernel+bounces-104586-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Fri, 15 Mar 2024 16:41:34 +0100
From: Joel Granados <j.granados@samsung.com>
To: Thomas =?utf-8?Q?Wei=C3=9Fschuh?= <linux@weissschuh.net>
CC: Luis Chamberlain <mcgrof@kernel.org>, Kees Cook <keescook@chromium.org>,
	"David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	<linux-kernel@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>,
	<netdev@vger.kernel.org>
Subject: Re: [PATCH v2] sysctl: drop unused argument set_ownership()::table
Message-ID: <20240315154134.5qfq6wucxid5kfmc@joelS2.panther.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg="pgp-sha512";
	protocol="application/pgp-signature"; boundary="r746sdyatcsm2m4p"
Content-Disposition: inline
In-Reply-To: <20240223-sysctl-const-ownership-v2-1-f9ba1795aaf2@weissschuh.net>
CMS-TYPE: 201P
References: <CGME20240223155011eucas1p245e00a3dfd23f72997dac4952ebaeebf@eucas1p2.samsung.com>
	<20240223-sysctl-const-ownership-v2-1-f9ba1795aaf2@weissschuh.net>

--r746sdyatcsm2m4p
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hey Thomas

Did you forget to compile? I'm seeing the following error when I
compile:

  ...
  ../fs/proc/proc_sysctl.c: In function =E2=80=98proc_sys_make_inode=E2=80=
=99:
  ../fs/proc/proc_sysctl.c:483:43: error: passing argument 2 of =E2=80=98ro=
ot->set_ownership=E2=80=99 from incompatible pointer type [-Werror=3Dincomp=
atible-pointer-types]
    483 |                 root->set_ownership(head, table, &inode->i_uid, &=
inode->i_gid);
        |                                           ^~~~~
        |                                           |
        |                                           struct ctl_table *
  ../fs/proc/proc_sysctl.c:483:43: note: expected =E2=80=98kuid_t *=E2=80=
=99 but argument is of type =E2=80=98struct ctl_table *=E2=80=99
  ../fs/proc/proc_sysctl.c:483:50: error: passing argument 3 of =E2=80=98ro=
ot->set_ownership=E2=80=99 from incompatible pointer type [-Werror=3Dincomp=
atible-pointer-types]
    483 |                 root->set_ownership(head, table, &inode->i_uid, &=
inode->i_gid);
        |                                                  ^~~~~~~~~~~~~
        |                                                  |
        |                                                  kuid_t *
  ../fs/proc/proc_sysctl.c:483:50: note: expected =E2=80=98kgid_t *=E2=80=
=99 but argument is of type =E2=80=98kuid_t *=E2=80=99
  ../fs/proc/proc_sysctl.c:483:17: error: too many arguments to function =
=E2=80=98root->set_ownership=E2=80=99
    483 |                 root->set_ownership(head, table, &inode->i_uid, &=
inode->i_gid);
        |                 ^~~~
  cc1: some warnings being treated as errors
  make[5]: *** [../scripts/Makefile.build:243: fs/proc/proc_sysctl.o] Error=
 1
    CC      fs/xfs/libxfs/xfs_dir2_node.o
  make[5]: *** Waiting for unfinished jobs....
  ...

I'm guessing its just a matter of removing the table arg from
proc_sys_make_inode?

Best

On Fri, Feb 23, 2024 at 04:50:00PM +0100, Thomas Wei=C3=9Fschuh wrote:
> The argument is never used and can be removed.
>=20
> In a future commit the sysctl core will only use
> "const struct ctl_table". Removing it here is a preparation for this
> consitifcation.
>=20
> The patch was created with the following coccinelle script:
>=20
>   @@
>   identifier func, head, table, uid, gid;
>   @@
>=20
>   void func(
>     struct ctl_table_header *head,
>   - struct ctl_table *table,
>     kuid_t *uid, kgid_t *gid)
>   { ... }
>=20
> The single changed location was validate through manual inspection and
> compilation.
>=20
> In addition, a search for 'set_ownership' was done over the full tree to
> look for places that were missed by coccinelle.
> None were found.
>=20
> Signed-off-by: Thomas Wei=C3=9Fschuh <linux@weissschuh.net>
> ---
> Changes in v2:
> - Rework commit message
> - Mention potential conflict with upcoming per-namespace kernel.pid_max
>   sysctl
> - Delete unused parameter table
> - Link to v1: https://lore.kernel.org/r/20231226-sysctl-const-ownership-v=
1-1-d78fdd744ba1@weissschuh.net
> ---
> The patch is meant to be merged via the sysctl tree.
>=20
> There is an upcoming series that will introduce a new implementation of
> .set_ownership which would need to be adapted [0].
> The adaption would be trivial as the 'table' parameter also unused
> there.
>=20
> This change was originally part of the sysctl-const series [1].
> To slim down that series and reduce the message load on other
> maintainers to a minimumble, submit this patch on its own.
>=20
> [0] https://lore.kernel.org/lkml/20240222160915.315255-1-aleksandr.mikhal=
itsyn@canonical.com/
> [1] https://lore.kernel.org/lkml/20231204-const-sysctl-v2-2-7a5060b11447@=
weissschuh.net/
> ---
>  include/linux/sysctl.h | 1 -
>  net/sysctl_net.c       | 1 -
>  2 files changed, 2 deletions(-)
>=20
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index ee7d33b89e9e..60333a6b9370 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -205,7 +205,6 @@ struct ctl_table_root {
>  	struct ctl_table_set default_set;
>  	struct ctl_table_set *(*lookup)(struct ctl_table_root *root);
>  	void (*set_ownership)(struct ctl_table_header *head,
> -			      struct ctl_table *table,
>  			      kuid_t *uid, kgid_t *gid);
>  	int (*permissions)(struct ctl_table_header *head, struct ctl_table *tab=
le);
>  };
> diff --git a/net/sysctl_net.c b/net/sysctl_net.c
> index 051ed5f6fc93..a0a7a79991f9 100644
> --- a/net/sysctl_net.c
> +++ b/net/sysctl_net.c
> @@ -54,7 +54,6 @@ static int net_ctl_permissions(struct ctl_table_header =
*head,
>  }
> =20
>  static void net_ctl_set_ownership(struct ctl_table_header *head,
> -				  struct ctl_table *table,
>  				  kuid_t *uid, kgid_t *gid)
>  {
>  	struct net *net =3D container_of(head->set, struct net, sysctls);
>=20
> ---
> base-commit: ffd2cb6b718e189e7e2d5d0c19c25611f92e061a
> change-id: 20231226-sysctl-const-ownership-ff75e67b4eea
>=20
> Best regards,
> --=20
> Thomas Wei=C3=9Fschuh <linux@weissschuh.net>
>=20

--=20

Joel Granados

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

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

iQGzBAABCgAdFiEErkcJVyXmMSXOyyeQupfNUreWQU8FAmX0bC0ACgkQupfNUreW
QU+whgv9En2sQX6VDbbF159wAFTNCqsVvsC8JU4vm6qZpr7kgyMgo9VxTd0P1HQ8
viituPEfsRbqPRQGKBUjfH3McRbMxpOw0KnuJIywAzbboecotskpVz9t4joaBugq
DmGBum1bCR/AQwPJlrjYb7FHq61alizsn+IclXAQFLPiHy6I5nJlr1fhIJOBzMOw
XzzeFQ4AOvuqJeazzcW92JDZ6PyRaRpXRBD0+S1TcdP+GgL5UyLyGyIKXSj7lYus
DEgQIinxF16/061BU+80O1XKG7s0gjwh/f5rflEMg7zwxwnOfJaqMKVmS+TR+mSl
IUlR09jEfASsCXDmqEFhlSFA1Bi5RSsJsAfBTkSn0DhzlrLD08rDqCa7Kqq6zfL4
/83ma83KSCPSpsApON5xA1hmDjT7gOhLSHtPo3IhgHmlJOwZWILdCf2CWHm4tX6s
p5d8MLqq10U+fu5XZ/zxOgnlYD186IwOiVMI2B5ixbA4pT6UOH6tLjG/VRB0Nr1A
J6DlNz1L
=RCo6
-----END PGP SIGNATURE-----

--r746sdyatcsm2m4p--