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

[Eric W. Biederman]

wenyang.linux@xxxxxxxxxxx writes:

> From: Wen Yang <wenyang.linux@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> Cc: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: Joel Granados <j.granados@xxxxxxxxxxx>
> Cc: Christian Brauner <brauner@xxxxxxxxxx>
> Cc: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
> Cc: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> ---
>  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"),