Received-SPF: pass (google.com: domain of linux-kernel+bounces-80069-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: wenyang.linux@foxmail.com
Cc: Luis Chamberlain <mcgrof@kernel.org>,  Kees Cook
 <keescook@chromium.org>,  Joel Granados <j.granados@samsung.com>,
  Christian Brauner <brauner@kernel.org>,  davem@davemloft.net,  David
 Ahern <dsahern@kernel.org>,  Eric Dumazet <edumazet@google.com>,  Jakub
 Kicinski <kuba@kernel.org>,  Paolo Abeni <pabeni@redhat.com>,  Shuah Khan
 <skhan@linuxfoundation.org>,  linux-kernel@vger.kernel.org
References: <20240225040538.845899-1-wenyang.linux@foxmail.com>
	<tencent_AA60553AD4DF41859328E9CACC367EFDCF08@qq.com>
Date: Sun, 25 Feb 2024 06:29:33 -0600
In-Reply-To: <tencent_AA60553AD4DF41859328E9CACC367EFDCF08@qq.com> (wenyang
	linux's message of "Sun, 25 Feb 2024 12:05:38 +0800")
Message-ID: <875xyczpzm.fsf@email.froward.int.ebiederm.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH 8/8] ucounts: delete these duplicate static variables
 ue_zero and ue_int_max

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"),