[PATCH 0/2] sysctl: Remove register_sysctl_table from sources

[PATCH 0/2] sysctl: Remove register_sysctl_table from sources

[Joel Granados]

This is part of the general push to deprecate register_sysctl_paths and
register_sysctl_table. This patchset completely removes register_sysctl_table
and replaces it with register_sysctl effectively transitioning 5 base paths
("kernel", "vm", "fs", "dev" and "debug") to the new call. Besides removing the
actuall function, I also removed it from the checks done in check-sysctl-docs.

Testing for this change was done in the same way as with previous sysctl
replacement patches: I made sure that the result of `find /proc/sys/ | sha1sum`
was the same before and after the patchset.

Have pushed this through 0-day. Waiting on results..

Feedback greatly appreciated.

Best
Joel

Joel Granados (2):
  sysctl: Refactor base paths registrations
  sysctl: Remove register_sysctl_table

 fs/proc/proc_sysctl.c     | 70 ---------------------------------------
 fs/sysctls.c              |  9 +++--
 include/linux/sysctl.h    | 23 -------------
 kernel/sysctl.c           | 13 +++-----
 scripts/check-sysctl-docs | 10 ------
 5 files changed, 10 insertions(+), 115 deletions(-)

-- 
2.30.2

[PATCH 1/2] sysctl: Refactor base paths registrations

[Joel Granados]

This is part of the general push to deprecate register_sysctl_paths and
register_sysctl_table. The old way of doing this through
register_sysctl_base and DECLARE_SYSCTL_BASE macro is replaced with a
call to register_sysctl. The 5 base paths affected are: "kernel", "vm",
"debug", "dev" and "fs".

We remove the register_sysctl_base function and the DECLARE_SYSCTL_BASE
macro since they are no longer needed.

In order to quickly acertain that the paths did not actually change I
executed `find /proc/sys/ | sha1sum` and made sure that the sha was the
same before and after the commit.

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 fs/sysctls.c           |  9 ++++++---
 include/linux/sysctl.h | 23 -----------------------
 kernel/sysctl.c        | 13 ++++---------
 3 files changed, 10 insertions(+), 35 deletions(-)

diff --git a/fs/sysctls.c b/fs/sysctls.c
index c701273c9432..228420f5fe1b 100644
--- a/fs/sysctls.c
+++ b/fs/sysctls.c
@@ -29,11 +29,14 @@ static struct ctl_table fs_shared_sysctls[] = {
 	{ }
 };
 
-DECLARE_SYSCTL_BASE(fs, fs_shared_sysctls);
-
 static int __init init_fs_sysctls(void)
 {
-	return register_sysctl_base(fs);
+	/*
+	 * We do not check the return code for register_sysctl because the
+	 * original call to register_sysctl_base always returned 0.
+	 */
+	register_sysctl("fs", fs_shared_sysctls);
+	return 0;
 }
 
 early_initcall(init_fs_sysctls);
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 218e56a26fb0..653b66c762b1 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -197,20 +197,6 @@ struct ctl_path {
 
 #ifdef CONFIG_SYSCTL
 
-#define DECLARE_SYSCTL_BASE(_name, _table)				\
-static struct ctl_table _name##_base_table[] = {			\
-	{								\
-		.procname	= #_name,				\
-		.mode		= 0555,					\
-		.child		= _table,				\
-	},								\
-	{ },								\
-}
-
-extern int __register_sysctl_base(struct ctl_table *base_table);
-
-#define register_sysctl_base(_name) __register_sysctl_base(_name##_base_table)
-
 void proc_sys_poll_notify(struct ctl_table_poll *poll);
 
 extern void setup_sysctl_set(struct ctl_table_set *p,
@@ -247,15 +233,6 @@ extern struct ctl_table sysctl_mount_point[];
 
 #else /* CONFIG_SYSCTL */
 
-#define DECLARE_SYSCTL_BASE(_name, _table)
-
-static inline int __register_sysctl_base(struct ctl_table *base_table)
-{
-	return 0;
-}
-
-#define register_sysctl_base(table) __register_sysctl_base(table)
-
 static inline void register_sysctl_init(const char *path, struct ctl_table *table)
 {
 }
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index bfe53e835524..f784b0fe5689 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2348,17 +2348,12 @@ static struct ctl_table dev_table[] = {
 	{ }
 };
 
-DECLARE_SYSCTL_BASE(kernel, kern_table);
-DECLARE_SYSCTL_BASE(vm, vm_table);
-DECLARE_SYSCTL_BASE(debug, debug_table);
-DECLARE_SYSCTL_BASE(dev, dev_table);
-
 int __init sysctl_init_bases(void)
 {
-	register_sysctl_base(kernel);
-	register_sysctl_base(vm);
-	register_sysctl_base(debug);
-	register_sysctl_base(dev);
+	register_sysctl("kernel", kern_table);
+	register_sysctl("vm", vm_table);
+	register_sysctl("debug", debug_table);
+	register_sysctl("dev", dev_table);
 
 	return 0;
 }
-- 
2.30.2

Re: [PATCH 1/2] sysctl: Refactor base paths registrations

[Guenter Roeck]

On Thu, May 18, 2023 at 06:07:04PM +0200, Joel Granados wrote:
> This is part of the general push to deprecate register_sysctl_paths and
> register_sysctl_table. The old way of doing this through
> register_sysctl_base and DECLARE_SYSCTL_BASE macro is replaced with a
> call to register_sysctl. The 5 base paths affected are: "kernel", "vm",
> "debug", "dev" and "fs".
> 
> We remove the register_sysctl_base function and the DECLARE_SYSCTL_BASE
> macro since they are no longer needed.
> 
> In order to quickly acertain that the paths did not actually change I
> executed `find /proc/sys/ | sha1sum` and made sure that the sha was the
> same before and after the commit.
> 
> Signed-off-by: Joel Granados <j.granados@samsung.com>

This patch results in the following warning, seen across almost
all architectures.

sysctl table check failed: kernel/usermodehelper Not a file
sysctl table check failed: kernel/usermodehelper No proc_handler
sysctl table check failed: kernel/usermodehelper bogus .mode 0555
sysctl table check failed: kernel/keys Not a file
sysctl table check failed: kernel/keys No proc_handler
sysctl table check failed: kernel/keys bogus .mode 0555
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.4.0-rc3-next-20230524 #1
Stack : ffffffff 801aed28 80e44644 00000004 81946ba4 00000000 810d3db4 b782f641
        810f0000 81269940 810f0000 810fa610 811bb193 00000001 810d3d58 00000000
        00000000 00000000 810099d8 000000f2 00000001 000000f3 00000000 00000000
        ffffffff 00000002 00000000 fff80000 810f0000 810099d8 00000001 ffffffea
        8013bab4 8113bb14 8013caa8 80fe0000 00000000 807b9d54 00000000 81270000
        ...
Call Trace:
[<8010a558>] show_stack+0x38/0x118
[<80d67edc>] dump_stack_lvl+0xa4/0xf0
[<8039c8e0>] __register_sysctl_table+0x5b4/0x7a0
[<811e55e4>] __register_sysctl_init+0x30/0x68
[<811d5164>] sysctl_init_bases+0x24/0x88
[<811e517c>] proc_root_init+0x94/0xa8
[<811ccebc>] start_kernel+0x704/0x740

failed when register_sysctl kern_table to kernel

Reverting this patch alone results in build failures. Reverting this patch
as well as the second patch in the series (to avoid the build failures)
fixes the problem.

Guenter

---
bisect log:

# bad: [cf09e328589a2ed7f6c8d90f2edb697fb4f8a96b] Add linux-next specific files for 20230524
# good: [44c026a73be8038f03dbdeef028b642880cf1511] Linux 6.4-rc3
git bisect start 'HEAD' 'v6.4-rc3'
# good: [a20d8ab9e26daaeeaf971139b736981cf164ab0a] Merge branch 'for-linux-next' of git://anongit.freedesktop.org/drm/drm-misc
git bisect good a20d8ab9e26daaeeaf971139b736981cf164ab0a
# good: [2714032dfd641b22695e14efd5f9dff08a5e3245] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git
git bisect good 2714032dfd641b22695e14efd5f9dff08a5e3245
# good: [b2bc2854ec87557033538aa9290f70b9141a6653] Merge branch 'for-leds-next' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git
git bisect good b2bc2854ec87557033538aa9290f70b9141a6653
# good: [26931c8431566f9bec7d57512e4cad8ebaeb024f] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git
git bisect good 26931c8431566f9bec7d57512e4cad8ebaeb024f
# good: [669623b562b5cd308eaa58eabe8c72007dbb37e2] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git
git bisect good 669623b562b5cd308eaa58eabe8c72007dbb37e2
# good: [a0ef85b20ffa65a89dc79b0a22edb80a88199939] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching
git bisect good a0ef85b20ffa65a89dc79b0a22edb80a88199939
# bad: [a64335537001eb6af6e57a82317985441dafe4e7] Merge branch 'sysctl-next' of git://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git
git bisect bad a64335537001eb6af6e57a82317985441dafe4e7
# good: [ba3ad1554b569888f4a63a2e4a16a9009fdedd4e] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
git bisect good ba3ad1554b569888f4a63a2e4a16a9009fdedd4e
# good: [3133f141259d7f6e0729db272f22724614472661] Merge branch 'slab/for-6.5/prandom' into slab/for-next
git bisect good 3133f141259d7f6e0729db272f22724614472661
# good: [2716d45c6fdc1ae06e83db28f58f55f7e9415643] sysctl: stop exporting register_sysctl_table
git bisect good 2716d45c6fdc1ae06e83db28f58f55f7e9415643
# good: [8ccb380db3a4bcef9ade852da8b33fdcea01c8a5] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git
git bisect good 8ccb380db3a4bcef9ade852da8b33fdcea01c8a5
# bad: [ee996cff1fb203bffc8de5c87cab6056d60df71d] sysctl: Remove register_sysctl_table
git bisect bad ee996cff1fb203bffc8de5c87cab6056d60df71d
# bad: [7eec88986dce2d85012fbe516def7a2d7d77735c] sysctl: Refactor base paths registrations
git bisect bad 7eec88986dce2d85012fbe516def7a2d7d77735c
# first bad commit: [7eec88986dce2d85012fbe516def7a2d7d77735c] sysctl: Refactor base paths registrations

Re: [PATCH 1/2] sysctl: Refactor base paths registrations

[Luis Chamberlain]

On Wed, May 24, 2023 at 06:29:06AM -0700, Guenter Roeck wrote:
> On Thu, May 18, 2023 at 06:07:04PM +0200, Joel Granados wrote:
> > This is part of the general push to deprecate register_sysctl_paths and
> > register_sysctl_table. The old way of doing this through
> > register_sysctl_base and DECLARE_SYSCTL_BASE macro is replaced with a
> > call to register_sysctl. The 5 base paths affected are: "kernel", "vm",
> > "debug", "dev" and "fs".
> > 
> > We remove the register_sysctl_base function and the DECLARE_SYSCTL_BASE
> > macro since they are no longer needed.
> > 
> > In order to quickly acertain that the paths did not actually change I
> > executed `find /proc/sys/ | sha1sum` and made sure that the sha was the
> > same before and after the commit.
> > 
> > Signed-off-by: Joel Granados <j.granados@samsung.com>
> 
> This patch results in the following warning, seen across almost
> all architectures.
> 
> sysctl table check failed: kernel/usermodehelper Not a file
> sysctl table check failed: kernel/usermodehelper No proc_handler
> sysctl table check failed: kernel/usermodehelper bogus .mode 0555
> sysctl table check failed: kernel/keys Not a file
> sysctl table check failed: kernel/keys No proc_handler
> sysctl table check failed: kernel/keys bogus .mode 0555
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.4.0-rc3-next-20230524 #1
> Stack : ffffffff 801aed28 80e44644 00000004 81946ba4 00000000 810d3db4 b782f641
>         810f0000 81269940 810f0000 810fa610 811bb193 00000001 810d3d58 00000000
>         00000000 00000000 810099d8 000000f2 00000001 000000f3 00000000 00000000
>         ffffffff 00000002 00000000 fff80000 810f0000 810099d8 00000001 ffffffea
>         8013bab4 8113bb14 8013caa8 80fe0000 00000000 807b9d54 00000000 81270000
>         ...
> Call Trace:
> [<8010a558>] show_stack+0x38/0x118
> [<80d67edc>] dump_stack_lvl+0xa4/0xf0
> [<8039c8e0>] __register_sysctl_table+0x5b4/0x7a0
> [<811e55e4>] __register_sysctl_init+0x30/0x68
> [<811d5164>] sysctl_init_bases+0x24/0x88
> [<811e517c>] proc_root_init+0x94/0xa8
> [<811ccebc>] start_kernel+0x704/0x740
> 
> failed when register_sysctl kern_table to kernel
> 
> Reverting this patch alone results in build failures. Reverting this patch
> as well as the second patch in the series (to avoid the build failures)
> fixes the problem.
> 
> Guenter

Thanks Guenter! The issue has been fixed on sysct-next, and I suppose
the fix will get into linux-next as of tomorrow.

 Luis

[PATCH 2/2] sysctl: Remove register_sysctl_table

[Joel Granados]

This is part of the general push to deprecate register_sysctl_paths and
register_sysctl_table. After removing all the calling functions, we
remove both the register_sysctl_table function and the documentation
check that appeared in check-sysctl-docs awk script.

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 fs/proc/proc_sysctl.c     | 70 ---------------------------------------
 scripts/check-sysctl-docs | 10 ------
 2 files changed, 80 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index f8f19e000d76..7bc7d3c3a215 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1574,76 +1574,6 @@ static int register_leaf_sysctl_tables(const char *path, char *pos,
 	return err;
 }
 
-/**
- * register_sysctl_table - register a sysctl table hierarchy
- * @table: the top-level table structure
- *
- * Register a sysctl table hierarchy. @table should be a filled in ctl_table
- * array. A completely 0 filled entry terminates the table.
- * We are slowly deprecating this call so avoid its use.
- */
-static struct ctl_table_header *register_sysctl_table(struct ctl_table *table)
-{
-	struct ctl_table *ctl_table_arg = table;
-	int nr_subheaders = count_subheaders(table);
-	struct ctl_table_header *header = NULL, **subheaders, **subheader;
-	char *new_path, *pos;
-
-	pos = new_path = kmalloc(PATH_MAX, GFP_KERNEL);
-	if (!new_path)
-		return NULL;
-
-	pos[0] = '\0';
-	while (table->procname && table->child && !table[1].procname) {
-		pos = append_path(new_path, pos, table->procname);
-		if (!pos)
-			goto out;
-		table = table->child;
-	}
-	if (nr_subheaders == 1) {
-		header = __register_sysctl_table(&sysctl_table_root.default_set, new_path, table);
-		if (header)
-			header->ctl_table_arg = ctl_table_arg;
-	} else {
-		header = kzalloc(sizeof(*header) +
-				 sizeof(*subheaders)*nr_subheaders, GFP_KERNEL);
-		if (!header)
-			goto out;
-
-		subheaders = (struct ctl_table_header **) (header + 1);
-		subheader = subheaders;
-		header->ctl_table_arg = ctl_table_arg;
-
-		if (register_leaf_sysctl_tables(new_path, pos, &subheader,
-						&sysctl_table_root.default_set, table))
-			goto err_register_leaves;
-	}
-
-out:
-	kfree(new_path);
-	return header;
-
-err_register_leaves:
-	while (subheader > subheaders) {
-		struct ctl_table_header *subh = *(--subheader);
-		struct ctl_table *table = subh->ctl_table_arg;
-		unregister_sysctl_table(subh);
-		kfree(table);
-	}
-	kfree(header);
-	header = NULL;
-	goto out;
-}
-
-int __register_sysctl_base(struct ctl_table *base_table)
-{
-	struct ctl_table_header *hdr;
-
-	hdr = register_sysctl_table(base_table);
-	kmemleak_not_leak(hdr);
-	return 0;
-}
-
 static void put_links(struct ctl_table_header *header)
 {
 	struct ctl_table_set *root_set = &sysctl_table_root.default_set;
diff --git a/scripts/check-sysctl-docs b/scripts/check-sysctl-docs
index edc9a629d79e..4f163e0bf6a4 100755
--- a/scripts/check-sysctl-docs
+++ b/scripts/check-sysctl-docs
@@ -146,16 +146,6 @@ curtable && /\.procname[\t ]*=[\t ]*".+"/ {
     children[curtable][curentry] = child
 }
 
-/register_sysctl_table\(.*\)/ {
-    match($0, /register_sysctl_table\(([^)]+)\)/, tables)
-    if (debug) print "Registering table " tables[1]
-    if (children[tables[1]][table]) {
-	for (entry in entries[children[tables[1]][table]]) {
-	    printentry(entry)
-	}
-    }
-}
-
 END {
     for (entry in documented) {
 	if (!seen[entry]) {
-- 
2.30.2

Re: [PATCH 0/2] sysctl: Remove register_sysctl_table from sources

[Luis Chamberlain]

On Thu, May 18, 2023 at 06:07:03PM +0200, Joel Granados wrote:
> This is part of the general push to deprecate register_sysctl_paths and
> register_sysctl_table. This patchset completely removes register_sysctl_table
> and replaces it with register_sysctl effectively transitioning 5 base paths
> ("kernel", "vm", "fs", "dev" and "debug") to the new call. Besides removing the
> actuall function, I also removed it from the checks done in check-sysctl-docs.
> 
> Testing for this change was done in the same way as with previous sysctl
> replacement patches: I made sure that the result of `find /proc/sys/ | sha1sum`
> was the same before and after the patchset.
> 
> Have pushed this through 0-day. Waiting on results..
> 
> Feedback greatly appreciated.

Thanks so much! I merged this to sysctl-testing as build tests are ongoing. But
I incorporated these minor changes to your first patch as register_sysctl_init()
is more obvious about when we cannot care about the return value.

If the build tests come through I'll push to sysctl-next.

diff --git a/fs/sysctls.c b/fs/sysctls.c
index 228420f5fe1b..76a0aee8c229 100644
--- a/fs/sysctls.c
+++ b/fs/sysctls.c
@@ -31,11 +31,7 @@ static struct ctl_table fs_shared_sysctls[] = {
 
 static int __init init_fs_sysctls(void)
 {
-	/*
-	 * We do not check the return code for register_sysctl because the
-	 * original call to register_sysctl_base always returned 0.
-	 */
-	register_sysctl("fs", fs_shared_sysctls);
+	register_sysctl_init("fs", fs_shared_sysctls);
 	return 0;
 }
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index f784b0fe5689..fa2aa8bd32b6 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2350,10 +2350,10 @@ static struct ctl_table dev_table[] = {
 
 int __init sysctl_init_bases(void)
 {
-	register_sysctl("kernel", kern_table);
-	register_sysctl("vm", vm_table);
-	register_sysctl("debug", debug_table);
-	register_sysctl("dev", dev_table);
+	register_sysctl_init("kernel", kern_table);
+	register_sysctl_init("vm", vm_table);
+	register_sysctl_init("debug", debug_table);
+	register_sysctl_init("dev", dev_table);
 
 	return 0;
 }

Re: [PATCH 0/2] sysctl: Remove register_sysctl_table from sources

[Luis Chamberlain]

On Thu, May 18, 2023 at 01:46:44PM -0700, Luis Chamberlain wrote:
> On Thu, May 18, 2023 at 06:07:03PM +0200, Joel Granados wrote:
> > This is part of the general push to deprecate register_sysctl_paths and
> > register_sysctl_table. This patchset completely removes register_sysctl_table
> > and replaces it with register_sysctl effectively transitioning 5 base paths
> > ("kernel", "vm", "fs", "dev" and "debug") to the new call. Besides removing the
> > actuall function, I also removed it from the checks done in check-sysctl-docs.
> > 
> > Testing for this change was done in the same way as with previous sysctl
> > replacement patches: I made sure that the result of `find /proc/sys/ | sha1sum`
> > was the same before and after the patchset.
> > 
> > Have pushed this through 0-day. Waiting on results..
> > 
> > Feedback greatly appreciated.
> 
> Thanks so much! I merged this to sysctl-testing as build tests are ongoing. But
> I incorporated these minor changes to your first patch as register_sysctl_init()
> is more obvious about when we cannot care about the return value.
> 
> If the build tests come through I'll push to sysctl-next.
> 

I also had to apply this (yay more nuking):

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 7bc7d3c3a215..8873812d22f3 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1466,19 +1466,6 @@ void __init __register_sysctl_init(const char *path, struct ctl_table *table,
 	kmemleak_not_leak(hdr);
 }
 
-static char *append_path(const char *path, char *pos, const char *name)
-{
-	int namelen;
-	namelen = strlen(name);
-	if (((pos - path) + namelen + 2) >= PATH_MAX)
-		return NULL;
-	memcpy(pos, name, namelen);
-	pos[namelen] = '/';
-	pos[namelen + 1] = '\0';
-	pos += namelen + 1;
-	return pos;
-}
-
 static int count_subheaders(struct ctl_table *table)
 {
 	int has_files = 0;
@@ -1498,82 +1485,6 @@ static int count_subheaders(struct ctl_table *table)
 	return nr_subheaders + has_files;
 }
 
-static int register_leaf_sysctl_tables(const char *path, char *pos,
-	struct ctl_table_header ***subheader, struct ctl_table_set *set,
-	struct ctl_table *table)
-{
-	struct ctl_table *ctl_table_arg = NULL;
-	struct ctl_table *entry, *files;
-	int nr_files = 0;
-	int nr_dirs = 0;
-	int err = -ENOMEM;
-
-	list_for_each_table_entry(entry, table) {
-		if (entry->child)
-			nr_dirs++;
-		else
-			nr_files++;
-	}
-
-	files = table;
-	/* If there are mixed files and directories we need a new table */
-	if (nr_dirs && nr_files) {
-		struct ctl_table *new;
-		files = kcalloc(nr_files + 1, sizeof(struct ctl_table),
-				GFP_KERNEL);
-		if (!files)
-			goto out;
-
-		ctl_table_arg = files;
-		new = files;
-
-		list_for_each_table_entry(entry, table) {
-			if (entry->child)
-				continue;
-			*new = *entry;
-			new++;
-		}
-	}
-
-	/* Register everything except a directory full of subdirectories */
-	if (nr_files || !nr_dirs) {
-		struct ctl_table_header *header;
-		header = __register_sysctl_table(set, path, files);
-		if (!header) {
-			kfree(ctl_table_arg);
-			goto out;
-		}
-
-		/* Remember if we need to free the file table */
-		header->ctl_table_arg = ctl_table_arg;
-		**subheader = header;
-		(*subheader)++;
-	}
-
-	/* Recurse into the subdirectories. */
-	list_for_each_table_entry(entry, table) {
-		char *child_pos;
-
-		if (!entry->child)
-			continue;
-
-		err = -ENAMETOOLONG;
-		child_pos = append_path(path, pos, entry->procname);
-		if (!child_pos)
-			goto out;
-
-		err = register_leaf_sysctl_tables(path, child_pos, subheader,
-						  set, entry->child);
-		pos[0] = '\0';
-		if (err)
-			goto out;
-	}
-	err = 0;
-out:
-	/* On failure our caller will unregister all registered subheaders */
-	return err;
-}
-
 static void put_links(struct ctl_table_header *header)
 {
 	struct ctl_table_set *root_set = &sysctl_table_root.default_set;

Re: [PATCH 0/2] sysctl: Remove register_sysctl_table from sources

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 4470 bytes --]

On Thu, May 18, 2023 at 05:26:34PM -0700, Luis Chamberlain wrote:
> On Thu, May 18, 2023 at 01:46:44PM -0700, Luis Chamberlain wrote:
> > On Thu, May 18, 2023 at 06:07:03PM +0200, Joel Granados wrote:
> > > This is part of the general push to deprecate register_sysctl_paths and
> > > register_sysctl_table. This patchset completely removes register_sysctl_table
> > > and replaces it with register_sysctl effectively transitioning 5 base paths
> > > ("kernel", "vm", "fs", "dev" and "debug") to the new call. Besides removing the
> > > actuall function, I also removed it from the checks done in check-sysctl-docs.
> > > 
> > > Testing for this change was done in the same way as with previous sysctl
> > > replacement patches: I made sure that the result of `find /proc/sys/ | sha1sum`
> > > was the same before and after the patchset.
> > > 
> > > Have pushed this through 0-day. Waiting on results..
> > > 
> > > Feedback greatly appreciated.
> > 
> > Thanks so much! I merged this to sysctl-testing as build tests are ongoing. But
> > I incorporated these minor changes to your first patch as register_sysctl_init()
> > is more obvious about when we cannot care about the return value.

nice! thx.

> > 
> > If the build tests come through I'll push to sysctl-next.
> > 
> 
> I also had to apply this (yay more nuking):
Indeed. I just saw the results of 0-day and there was a warning
regarding these functions. Thx again.

best
joel
> 
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index 7bc7d3c3a215..8873812d22f3 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -1466,19 +1466,6 @@ void __init __register_sysctl_init(const char *path, struct ctl_table *table,
>  	kmemleak_not_leak(hdr);
>  }
>  
> -static char *append_path(const char *path, char *pos, const char *name)
> -{
> -	int namelen;
> -	namelen = strlen(name);
> -	if (((pos - path) + namelen + 2) >= PATH_MAX)
> -		return NULL;
> -	memcpy(pos, name, namelen);
> -	pos[namelen] = '/';
> -	pos[namelen + 1] = '\0';
> -	pos += namelen + 1;
> -	return pos;
> -}
> -
>  static int count_subheaders(struct ctl_table *table)
>  {
>  	int has_files = 0;
> @@ -1498,82 +1485,6 @@ static int count_subheaders(struct ctl_table *table)
>  	return nr_subheaders + has_files;
>  }
>  
> -static int register_leaf_sysctl_tables(const char *path, char *pos,
> -	struct ctl_table_header ***subheader, struct ctl_table_set *set,
> -	struct ctl_table *table)
> -{
> -	struct ctl_table *ctl_table_arg = NULL;
> -	struct ctl_table *entry, *files;
> -	int nr_files = 0;
> -	int nr_dirs = 0;
> -	int err = -ENOMEM;
> -
> -	list_for_each_table_entry(entry, table) {
> -		if (entry->child)
> -			nr_dirs++;
> -		else
> -			nr_files++;
> -	}
> -
> -	files = table;
> -	/* If there are mixed files and directories we need a new table */
> -	if (nr_dirs && nr_files) {
> -		struct ctl_table *new;
> -		files = kcalloc(nr_files + 1, sizeof(struct ctl_table),
> -				GFP_KERNEL);
> -		if (!files)
> -			goto out;
> -
> -		ctl_table_arg = files;
> -		new = files;
> -
> -		list_for_each_table_entry(entry, table) {
> -			if (entry->child)
> -				continue;
> -			*new = *entry;
> -			new++;
> -		}
> -	}
> -
> -	/* Register everything except a directory full of subdirectories */
> -	if (nr_files || !nr_dirs) {
> -		struct ctl_table_header *header;
> -		header = __register_sysctl_table(set, path, files);
> -		if (!header) {
> -			kfree(ctl_table_arg);
> -			goto out;
> -		}
> -
> -		/* Remember if we need to free the file table */
> -		header->ctl_table_arg = ctl_table_arg;
> -		**subheader = header;
> -		(*subheader)++;
> -	}
> -
> -	/* Recurse into the subdirectories. */
> -	list_for_each_table_entry(entry, table) {
> -		char *child_pos;
> -
> -		if (!entry->child)
> -			continue;
> -
> -		err = -ENAMETOOLONG;
> -		child_pos = append_path(path, pos, entry->procname);
> -		if (!child_pos)
> -			goto out;
> -
> -		err = register_leaf_sysctl_tables(path, child_pos, subheader,
> -						  set, entry->child);
> -		pos[0] = '\0';
> -		if (err)
> -			goto out;
> -	}
> -	err = 0;
> -out:
> -	/* On failure our caller will unregister all registered subheaders */
> -	return err;
> -}
> -
>  static void put_links(struct ctl_table_header *header)
>  {
>  	struct ctl_table_set *root_set = &sysctl_table_root.default_set;

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]