Re: [PATCH] __register_sysctl_table: do not drop subdir

[Luis Chamberlain <mcgrof@kernel.org>]

Eric since you authored the code which this code claism to fix, your
review would be appreciated.

On Wed, May 27, 2020 at 01:48:48PM +0300, Boris Sukholitko wrote:
> Successful get_subdir returns dir with its header.nreg properly
> adjusted. No need to drop the dir in that case.

This commit log is not that clear to me, can you explain what happens
without this patch, and how critical it is to fix it. How did you
notice this issue? If you don't apply this patch what issue do you
see?

Do we test for it? Can we?

  Luis

> 
> Signed-off-by: Boris Sukholitko <boris.sukholitko@broadcom.com>
> Fixes: 7ec66d06362d (sysctl: Stop requiring explicit management of sysctl directories)
> ---
>  fs/proc/proc_sysctl.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index b6f5d459b087..6f237f0eb531 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -1286,8 +1286,8 @@ struct ctl_table_header *__register_sysctl_table(
>  {
>  	struct ctl_table_root *root = set->dir.header.root;
>  	struct ctl_table_header *header;
> +	struct ctl_dir *dir, *start_dir;
>  	const char *name, *nextname;
> -	struct ctl_dir *dir;
>  	struct ctl_table *entry;
>  	struct ctl_node *node;
>  	int nr_entries = 0;
> @@ -1307,6 +1307,7 @@ struct ctl_table_header *__register_sysctl_table(
>  
>  	spin_lock(&sysctl_lock);
>  	dir = &set->dir;
> +	start_dir = dir;
>  	/* Reference moved down the diretory tree get_subdir */
>  	dir->header.nreg++;
>  	spin_unlock(&sysctl_lock);
> @@ -1333,7 +1334,8 @@ struct ctl_table_header *__register_sysctl_table(
>  	if (insert_header(dir, header))
>  		goto fail_put_dir_locked;
>  
> -	drop_sysctl_table(&dir->header);
> +	if (start_dir == dir)
> +		drop_sysctl_table(&dir->header);
>  	spin_unlock(&sysctl_lock);
>  
>  	return header;
> -- 
> 2.23.1
>