linux-kernel - Re: [PATCH 0/2] sysctl: Remove register_sysctl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZGbCOjS1n6zV9ZGV@bombadil.infradead.org>
Date:   Thu, 18 May 2023 17:26:34 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Joel Granados <j.granados@...sung.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        linux-kernel@...r.kernel.org, Iurii Zaikin <yzaikin@...gle.com>,
        Sudip Mukherjee <sudipm.mukherjee@...il.com>,
        Christian Brauner <brauner@...nel.org>,
        linux-fsdevel@...r.kernel.org, Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH 0/2] sysctl: Remove register_sysctl_table from sources

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;