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

[Joel Granados]

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: signature.asc
Description: PGP signature