Re: [PATCH 8/8] ucounts: delete these duplicate static variables ue_zero and ue_int_max

["Eric W. Biederman" <ebiederm@xmission.com>]

wenyang.linux@foxmail.com writes:

> From: Wen Yang <wenyang.linux@foxmail.com>
>
> Since these static variables (ue_zero and ue_int_max) are only used for
> boundary checks and will not be changed, remove it and use the ones in
> our shared const array.

What happened to the plans to kill the shared const array?

You can still save a lot more by turning .extra1 and .extra2
into longs instead of keeping them as pointers and needing
constants to be pointed at somewhere.

As I recall the last version of this actually broke the code,
(but not on little endian).

This one if the constants are properly named looks better
than that, but I don't see any reason why you want shared
constants for such a handful of things.  Especially when
it has proven to be error prone in the past.

The only people I can see who find a significant benefit by
consolidating all of the constants into one place are people who know
how to stomp kernel memory.

Eric


>
> Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Joel Granados <j.granados@samsung.com>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Cc: linux-kernel@vger.kernel.org
> ---
>  kernel/ucount.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/ucount.c b/kernel/ucount.c
> index 4aa6166cb856..05bbba02ae4f 100644
> --- a/kernel/ucount.c
> +++ b/kernel/ucount.c
> @@ -58,17 +58,14 @@ static struct ctl_table_root set_root = {
>  	.permissions = set_permissions,
>  };
>  
> -static long ue_zero = 0;
> -static long ue_int_max = INT_MAX;
> -
>  #define UCOUNT_ENTRY(name)					\
>  	{							\
>  		.procname	= name,				\
>  		.maxlen		= sizeof(long),			\
>  		.mode		= 0644,				\
>  		.proc_handler	= proc_doulongvec_minmax,	\
> -		.extra1		= &ue_zero,			\
> -		.extra2		= &ue_int_max,			\
> +		.extra1		= SYSCTL_LONG_ZERO,		\
> +		.extra2		= SYSCTL_LONG_S32_MAX,		\
>  	}
>  static struct ctl_table user_table[] = {
>  	UCOUNT_ENTRY("max_user_namespaces"),