[PATCH 0/2] sysctl: move umh and keys sysctls

[PATCH 0/2] sysctl: move umh and keys sysctls

[Luis Chamberlain]

If you look at kernel/sysctl.c there are two sysctl arrays which
are declared in header files but registered with no good reason now
on kernel/sysctl.c instead of the place they belong. So just do
the registration where it belongs.

The penalty of this is just 66 bytes for moving both registrations
to its own file, but soon we'll be removing all sysctl empty entries
at each array, and we've already done tons of cleanup on fs/proc/proc_sysctl.c
which saved us hundreds of bytes so we have few karma points.

With this, we no now only have two sysctl arrays left to start clearing
up the kernel one and the vm one.

Luis Chamberlain (2):
  sysctl: move umh sysctl registration to its own file
  sysctl: move security keys sysctl registration to its own file

 include/linux/key.h    |  3 ---
 include/linux/umh.h    |  2 --
 kernel/sysctl.c        |  5 -----
 kernel/umh.c           | 11 ++++++++++-
 security/keys/sysctl.c |  7 +++++++
 5 files changed, 17 insertions(+), 11 deletions(-)

-- 
2.39.2

[PATCH 1/2] sysctl: move umh sysctl registration to its own file

[Luis Chamberlain]

Move the umh sysctl registration to its own file, the array is
already there. We do this to remove the clutter out of kernel/sysctl.c
to avoid merge conflicts.

This also lets the sysctls not be built at all now when CONFIG_SYSCTL
is not enabled.

This has a small penalty of 23 bytes but soon we'll be removing
all the empty entries on sysctl arrays so just do this cleanup
now:

./scripts/bloat-o-meter vmlinux.base vmlinux.1
add/remove: 2/0 grow/shrink: 0/1 up/down: 49/-26 (23)
Function                                     old     new   delta
init_umh_sysctls                               -      33     +33
__pfx_init_umh_sysctls                         -      16     +16
sysctl_init_bases                            111      85     -26
Total: Before=21256914, After=21256937, chg +0.00%

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 include/linux/umh.h |  2 --
 kernel/sysctl.c     |  1 -
 kernel/umh.c        | 11 ++++++++++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/linux/umh.h b/include/linux/umh.h
index 5d1f6129b847..daa6a7048c11 100644
--- a/include/linux/umh.h
+++ b/include/linux/umh.h
@@ -42,8 +42,6 @@ call_usermodehelper_setup(const char *path, char **argv, char **envp,
 extern int
 call_usermodehelper_exec(struct subprocess_info *info, int wait);
 
-extern struct ctl_table usermodehelper_table[];
-
 enum umh_disable_depth {
 	UMH_ENABLED = 0,
 	UMH_FREEZING,
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 241b817c0240..caf4a91522a1 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2322,7 +2322,6 @@ static struct ctl_table vm_table[] = {
 int __init sysctl_init_bases(void)
 {
 	register_sysctl_init("kernel", kern_table);
-	register_sysctl_init("kernel/usermodehelper", usermodehelper_table);
 #ifdef CONFIG_KEYS
 	register_sysctl_init("kernel/keys", key_sysctls);
 #endif
diff --git a/kernel/umh.c b/kernel/umh.c
index 60aa9e764a38..41088c5c39fd 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -544,7 +544,8 @@ static int proc_cap_handler(struct ctl_table *table, int write,
 	return 0;
 }
 
-struct ctl_table usermodehelper_table[] = {
+#if defined(CONFIG_SYSCTL)
+static struct ctl_table usermodehelper_table[] = {
 	{
 		.procname	= "bset",
 		.data		= &usermodehelper_bset,
@@ -561,3 +562,11 @@ struct ctl_table usermodehelper_table[] = {
 	},
 	{ }
 };
+
+static int __init init_umh_sysctls(void)
+{
+	register_sysctl_init("kernel/usermodehelper", usermodehelper_table);
+	return 0;
+}
+early_initcall(init_umh_sysctls);
+#endif /* CONFIG_SYSCTL */
-- 
2.39.2

Re: [PATCH 1/2] sysctl: move umh sysctl registration to its own file

[Jarkko Sakkinen]

On Wed May 31, 2023 at 2:29 AM EEST, Luis Chamberlain wrote:
> Move the umh sysctl registration to its own file, the array is
> already there. We do this to remove the clutter out of kernel/sysctl.c
> to avoid merge conflicts.
>
> This also lets the sysctls not be built at all now when CONFIG_SYSCTL
> is not enabled.
>
> This has a small penalty of 23 bytes but soon we'll be removing
> all the empty entries on sysctl arrays so just do this cleanup
> now:
>
> ./scripts/bloat-o-meter vmlinux.base vmlinux.1
> add/remove: 2/0 grow/shrink: 0/1 up/down: 49/-26 (23)
> Function                                     old     new   delta
> init_umh_sysctls                               -      33     +33
> __pfx_init_umh_sysctls                         -      16     +16
> sysctl_init_bases                            111      85     -26
> Total: Before=21256914, After=21256937, chg +0.00%
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  include/linux/umh.h |  2 --
>  kernel/sysctl.c     |  1 -
>  kernel/umh.c        | 11 ++++++++++-
>  3 files changed, 10 insertions(+), 4 deletions(-)

Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

[PATCH 2/2] sysctl: move security keys sysctl registration to its own file

[Luis Chamberlain]

The security keys sysctls are already declared on its own file,
just move the sysctl registration to its own file to help avoid
merge conflicts on sysctls.c, and help with clearing up sysctl.c
further.

This creates a small penalty of 23 bytes:

./scripts/bloat-o-meter vmlinux.1 vmlinux.2
add/remove: 2/0 grow/shrink: 0/1 up/down: 49/-26 (23)
Function                                     old     new   delta
init_security_keys_sysctls                     -      33     +33
__pfx_init_security_keys_sysctls               -      16     +16
sysctl_init_bases                             85      59     -26
Total: Before=21256937, After=21256960, chg +0.00%

But soon we'll be saving tons of bytes anyway, as we modify the
sysctl registrations to use ARRAY_SIZE and so we get rid of all the
empty array elements so let's just clean this up now.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 include/linux/key.h    | 3 ---
 kernel/sysctl.c        | 4 ----
 security/keys/sysctl.c | 7 +++++++
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index 8dc7f7c3088b..938d7ecfb495 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -490,9 +490,6 @@ do {									\
 	rcu_assign_pointer((KEY)->payload.rcu_data0, (PAYLOAD));	\
 } while (0)
 
-#ifdef CONFIG_SYSCTL
-extern struct ctl_table key_sysctls[];
-#endif
 /*
  * the userspace interface
  */
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index caf4a91522a1..48046932d573 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2322,10 +2322,6 @@ static struct ctl_table vm_table[] = {
 int __init sysctl_init_bases(void)
 {
 	register_sysctl_init("kernel", kern_table);
-#ifdef CONFIG_KEYS
-	register_sysctl_init("kernel/keys", key_sysctls);
-#endif
-
 	register_sysctl_init("vm", vm_table);
 
 	return 0;
diff --git a/security/keys/sysctl.c b/security/keys/sysctl.c
index b46b651b3c4c..b72b82bb20c6 100644
--- a/security/keys/sysctl.c
+++ b/security/keys/sysctl.c
@@ -68,3 +68,10 @@ struct ctl_table key_sysctls[] = {
 #endif
 	{ }
 };
+
+static int __init init_security_keys_sysctls(void)
+{
+	register_sysctl_init("kernel/keys", key_sysctls);
+	return 0;
+}
+early_initcall(init_security_keys_sysctls);
-- 
2.39.2

Re: [PATCH 2/2] sysctl: move security keys sysctl registration to its own file

[Paul Moore]

On Tue, May 30, 2023 at 7:29 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> The security keys sysctls are already declared on its own file,
> just move the sysctl registration to its own file to help avoid
> merge conflicts on sysctls.c, and help with clearing up sysctl.c
> further.
>
> This creates a small penalty of 23 bytes:
>
> ./scripts/bloat-o-meter vmlinux.1 vmlinux.2
> add/remove: 2/0 grow/shrink: 0/1 up/down: 49/-26 (23)
> Function                                     old     new   delta
> init_security_keys_sysctls                     -      33     +33
> __pfx_init_security_keys_sysctls               -      16     +16
> sysctl_init_bases                             85      59     -26
> Total: Before=21256937, After=21256960, chg +0.00%
>
> But soon we'll be saving tons of bytes anyway, as we modify the
> sysctl registrations to use ARRAY_SIZE and so we get rid of all the
> empty array elements so let's just clean this up now.
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  include/linux/key.h    | 3 ---
>  kernel/sysctl.c        | 4 ----
>  security/keys/sysctl.c | 7 +++++++
>  3 files changed, 7 insertions(+), 7 deletions(-)

Ultimately I'll leave the ACK to David or Jarkko, but this looks
reasonable to me.

Reviewed-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com

Re: [PATCH 2/2] sysctl: move security keys sysctl registration to its own file

[Luis Chamberlain]

On Wed, May 31, 2023 at 05:20:46PM -0400, Paul Moore wrote:
> On Tue, May 30, 2023 at 7:29 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> >
> > The security keys sysctls are already declared on its own file,
> > just move the sysctl registration to its own file to help avoid
> > merge conflicts on sysctls.c, and help with clearing up sysctl.c
> > further.
> >
> > This creates a small penalty of 23 bytes:
> >
> > ./scripts/bloat-o-meter vmlinux.1 vmlinux.2
> > add/remove: 2/0 grow/shrink: 0/1 up/down: 49/-26 (23)
> > Function                                     old     new   delta
> > init_security_keys_sysctls                     -      33     +33
> > __pfx_init_security_keys_sysctls               -      16     +16
> > sysctl_init_bases                             85      59     -26
> > Total: Before=21256937, After=21256960, chg +0.00%
> >
> > But soon we'll be saving tons of bytes anyway, as we modify the
> > sysctl registrations to use ARRAY_SIZE and so we get rid of all the
> > empty array elements so let's just clean this up now.
> >
> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> > ---
> >  include/linux/key.h    | 3 ---
> >  kernel/sysctl.c        | 4 ----
> >  security/keys/sysctl.c | 7 +++++++
> >  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> Ultimately I'll leave the ACK to David or Jarkko, but this looks
> reasonable to me.
> 
> Reviewed-by: Paul Moore <paul@paul-moore.com>

I've queued this onto sysctl-next as I haven't seen any complaints.
I can drop it if there are complaints or regressions reported by
folks on linux-next.

  Luis

Re: [PATCH 2/2] sysctl: move security keys sysctl registration to its own file

[Jarkko Sakkinen]

On Wed May 31, 2023 at 2:29 AM EEST, Luis Chamberlain wrote:
> The security keys sysctls are already declared on its own file,
> just move the sysctl registration to its own file to help avoid
> merge conflicts on sysctls.c, and help with clearing up sysctl.c
> further.
>
> This creates a small penalty of 23 bytes:
>
> ./scripts/bloat-o-meter vmlinux.1 vmlinux.2
> add/remove: 2/0 grow/shrink: 0/1 up/down: 49/-26 (23)
> Function                                     old     new   delta
> init_security_keys_sysctls                     -      33     +33
> __pfx_init_security_keys_sysctls               -      16     +16
> sysctl_init_bases                             85      59     -26
> Total: Before=21256937, After=21256960, chg +0.00%
>
> But soon we'll be saving tons of bytes anyway, as we modify the
> sysctl registrations to use ARRAY_SIZE and so we get rid of all the
> empty array elements so let's just clean this up now.
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  include/linux/key.h    | 3 ---
>  kernel/sysctl.c        | 4 ----
>  security/keys/sysctl.c | 7 +++++++
>  3 files changed, 7 insertions(+), 7 deletions(-)

Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

Re: [PATCH 2/2] sysctl: move security keys sysctl registration to its own file

[David Howells]

Luis Chamberlain <mcgrof@kernel.org> wrote:

> The security keys sysctls are already declared on its own file,
> just move the sysctl registration to its own file to help avoid
> merge conflicts on sysctls.c, and help with clearing up sysctl.c
> further.
> 
> This creates a small penalty of 23 bytes:
> 
> ./scripts/bloat-o-meter vmlinux.1 vmlinux.2
> add/remove: 2/0 grow/shrink: 0/1 up/down: 49/-26 (23)
> Function                                     old     new   delta
> init_security_keys_sysctls                     -      33     +33
> __pfx_init_security_keys_sysctls               -      16     +16
> sysctl_init_bases                             85      59     -26
> Total: Before=21256937, After=21256960, chg +0.00%
> 
> But soon we'll be saving tons of bytes anyway, as we modify the
> sysctl registrations to use ARRAY_SIZE and so we get rid of all the
> empty array elements so let's just clean this up now.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>

Acked-by: David Howells <dhowells@redhat.com>