[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <875xyczpzm.fsf@email.froward.int.ebiederm.org>
Date: Sun, 25 Feb 2024 06:29:33 -0600
From: "Eric W. Biederman" <ebiederm@...ssion.com>
To: wenyang.linux@...mail.com
Cc: Luis Chamberlain <mcgrof@...nel.org>, Kees Cook
<keescook@...omium.org>, Joel Granados <j.granados@...sung.com>,
Christian Brauner <brauner@...nel.org>, davem@...emloft.net, David
Ahern <dsahern@...nel.org>, Eric Dumazet <edumazet@...gle.com>, Jakub
Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Shuah Khan
<skhan@...uxfoundation.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 8/8] ucounts: delete these duplicate static variables
ue_zero and ue_int_max
wenyang.linux@...mail.com writes:
> From: Wen Yang <wenyang.linux@...mail.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@...mail.com>
> Cc: Luis Chamberlain <mcgrof@...nel.org>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Joel Granados <j.granados@...sung.com>
> Cc: Christian Brauner <brauner@...nel.org>
> Cc: "Eric W. Biederman" <ebiederm@...ssion.com>
> Cc: Shuah Khan <skhan@...uxfoundation.org>
> Cc: linux-kernel@...r.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"),
Powered by blists - more mailing lists