linux-kernel - Re: [PATCH 4/9] sysctl: move maxolduid as a sysctl specific const

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]

Date:   Fri, 17 Dec 2021 17:15:01 +0100
From:   Mickaël Salaün <mic@...ikod.net>
To:     Luis Chamberlain <mcgrof@...nel.org>, akpm@...ux-foundation.org,
        viro@...iv.linux.org.uk, keescook@...omium.org, yzaikin@...gle.com,
        nixiaoming@...wei.com, ebiederm@...ssion.com, steve@....org,
        mcgrof@...badil.infradead.org, andriy.shevchenko@...ux.intel.com,
        jlayton@...nel.org, bfields@...ldses.org,
        Stephen Rothwell <sfr@...b.auug.org.au>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/9] sysctl: move maxolduid as a sysctl specific const

This patch introduces a bug in -next:

On 29/11/2021 21:55, Luis Chamberlain wrote:
> The maxolduid value is only shared for sysctl purposes for
> use on a max range. Just stuff this into our shared const
> array.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@...nel.org>
> ---
>   fs/proc/proc_sysctl.c  |  2 +-
>   include/linux/sysctl.h |  3 +++
>   kernel/sysctl.c        | 12 ++++--------
>   3 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index 7dec3d5a9ed4..675b625fa898 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -26,7 +26,7 @@ static const struct file_operations proc_sys_dir_file_operations;
>   static const struct inode_operations proc_sys_dir_operations;
>   
>   /* shared constants to be used in various sysctls */
> -const int sysctl_vals[] = { -1, 0, 1, 2, 4, 100, 200, 1000, 3000, INT_MAX };
> +const int sysctl_vals[] = { -1, 0, 1, 2, 4, 100, 200, 1000, 65535, INT_MAX };

The new SYSCTL_MAXOLDUID uses the index 10 of sysctl_vals[] but the same
commit replaces index 8 (SYSCTL_THREE_THOUSAND used by
vm.watermark_scale_factor) instead of adding a new entry.

It should be:
+const int sysctl_vals[] = { -1, 0, 1, 2, 4, 100, 200, 1000, 3000, 
INT_MAX, 65535 };



>   EXPORT_SYMBOL(sysctl_vals);
>   
>   const unsigned long sysctl_long_vals[] = { 0, 1, LONG_MAX };
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index 2de6d20d191b..bb921eb8a02d 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -49,6 +49,9 @@ struct ctl_dir;
>   #define SYSCTL_THREE_THOUSAND		((void *)&sysctl_vals[8])
>   #define SYSCTL_INT_MAX			((void *)&sysctl_vals[9])
>   
> +/* this is needed for the proc_dointvec_minmax for [fs_]overflow UID and GID */
> +#define SYSCTL_MAXOLDUID		((void *)&sysctl_vals[10])
> +
>   extern const int sysctl_vals[];
>   
>   #define SYSCTL_LONG_ZERO	((void *)&sysctl_long_vals[0])
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index dbd267d0f014..05d9dd85e17f 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -109,10 +109,6 @@ static const int six_hundred_forty_kb = 640 * 1024;
>   /* this is needed for the proc_doulongvec_minmax of vm_dirty_bytes */
>   static const unsigned long dirty_bytes_min = 2 * PAGE_SIZE;
>   
> -/* this is needed for the proc_dointvec_minmax for [fs_]overflow UID and GID */
> -static const int maxolduid = 65535;
> -/* minolduid is SYSCTL_ZERO */
> -
>   static const int ngroups_max = NGROUPS_MAX;
>   static const int cap_last_cap = CAP_LAST_CAP;
>   
> @@ -2126,7 +2122,7 @@ static struct ctl_table kern_table[] = {
>   		.mode		= 0644,
>   		.proc_handler	= proc_dointvec_minmax,
>   		.extra1		= SYSCTL_ZERO,
> -		.extra2		= (void *)&maxolduid,
> +		.extra2		= SYSCTL_MAXOLDUID,
>   	},
>   	{
>   		.procname	= "overflowgid",
> @@ -2135,7 +2131,7 @@ static struct ctl_table kern_table[] = {
>   		.mode		= 0644,
>   		.proc_handler	= proc_dointvec_minmax,
>   		.extra1		= SYSCTL_ZERO,
> -		.extra2		= (void *)&maxolduid,
> +		.extra2		= SYSCTL_MAXOLDUID,
>   	},
>   #ifdef CONFIG_S390
>   	{
> @@ -2907,7 +2903,7 @@ static struct ctl_table fs_table[] = {
>   		.mode		= 0644,
>   		.proc_handler	= proc_dointvec_minmax,
>   		.extra1		= SYSCTL_ZERO,
> -		.extra2		= (void *)&maxolduid,
> +		.extra2		= SYSCTL_MAXOLDUID,
>   	},
>   	{
>   		.procname	= "overflowgid",
> @@ -2916,7 +2912,7 @@ static struct ctl_table fs_table[] = {
>   		.mode		= 0644,
>   		.proc_handler	= proc_dointvec_minmax,
>   		.extra1		= SYSCTL_ZERO,
> -		.extra2		= (void *)&maxolduid,
> +		.extra2		= SYSCTL_MAXOLDUID,
>   	},
>   #ifdef CONFIG_FILE_LOCKING
>   	{
>