* [PATCH v2 0/9] encode the values directly in the table entry
@ 2024-03-09 10:31 ` wenyang.linux
2024-03-21 15:21 ` Joel Granados
0 siblings, 1 reply; 3+ messages in thread
From: wenyang.linux @ 2024-03-09 10:31 UTC (permalink / raw)
To: Eric W . Biederman, Luis Chamberlain, Kees Cook, Joel Granados,
Christian Brauner
Cc: Wen Yang, linux-kernel
From: Wen Yang <wenyang.linux@foxmail.com>
The boundary check of multiple modules uses these static variables (such as
two_five_five, n_65535, ue_int_max, etc), and they are also not changed.
Eric points out: "by turning .extra1 and .extra2 into longs instead of
keeping them as pointers and needing constants to be pointed at somewhere
... 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."
This patch series achieves direct encoding values in table entries and still
maintains compatibility with existing extra1/extra2 pointers.
Afterwards, we can remove these unnecessary static variables progressively and
also gradually kill the shared const array.
Wen Yang (9):
sysctl: support encoding values directly in the table entry
kernel/sysctl-test: add some kunit test cases for min/max detection
rxrpc: delete these unnecessary static variables n_65535, four,
max_500, etc
net: ipv6: delete these unnecessary static variables two_five_five and
minus_one
svcrdma: delete these unnecessary static variables min_ord, max_ord,
etc
sysctl: delete these unnecessary static variables i_zero and
i_one_hundred
epoll: delete these unnecessary static variables long_zero and
long_max
fs: inotify: delete these unnecessary static variables it_zero and
it_int_max
ucounts: delete these unnecessary static variables ue_zero and
ue_int_max
fs/eventpoll.c | 19 +-
fs/notify/inotify/inotify_user.c | 49 +++--
include/linux/sysctl.h | 108 ++++++++++-
kernel/sysctl-test.c | 300 +++++++++++++++++++++++++++++++
kernel/sysctl.c | 61 +++++--
kernel/ucount.c | 8 +-
lib/test_sysctl.c | 12 +-
net/ipv6/addrconf.c | 15 +-
net/rxrpc/sysctl.c | 169 ++++++++---------
net/sunrpc/xprtrdma/svc_rdma.c | 21 +--
10 files changed, 571 insertions(+), 191 deletions(-)
Cc: Eric W. Biederman <ebiederm@xmission.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: linux-kernel@vger.kernel.org
--
2.25.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 0/9] encode the values directly in the table entry
2024-03-09 10:31 ` [PATCH v2 0/9] encode the values directly in the table entry wenyang.linux
@ 2024-03-21 15:21 ` Joel Granados
2024-03-21 15:39 ` Wen Yang
0 siblings, 1 reply; 3+ messages in thread
From: Joel Granados @ 2024-03-21 15:21 UTC (permalink / raw)
To: wenyang.linux
Cc: Eric W . Biederman, Luis Chamberlain, Kees Cook,
Christian Brauner, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3292 bytes --]
On Sat, Mar 09, 2024 at 06:31:17PM +0800, wenyang.linux@foxmail.com wrote:
> From: Wen Yang <wenyang.linux@foxmail.com>
>
> The boundary check of multiple modules uses these static variables (such as
> two_five_five, n_65535, ue_int_max, etc), and they are also not changed.
This message is a bit cryptic. I had to do a fair amount of research to
get what you meant here. Having the context in front is OK with me, but
I would add a bit more information so the reader does not have to go to
the code and grep for the variables that you mean. Something like this:
"When using a sysctl proc_handler that requires a boundary check (like
proce_dointvec_minmax) it is common to use a const variable like n_65535
in net/rxrpc/sysctl.c or OTHER EXAMPLES...). This is suboptimal because
YOUR REASONS HERE"
>
> Eric points out: "by turning .extra1 and .extra2 into longs instead of
> keeping them as pointers and needing constants to be pointed at somewhere
> ... 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."
I think it would be better to just link to the lore discussion.
>
> This patch series achieves direct encoding values in table entries and still
> maintains compatibility with existing extra1/extra2 pointers.
> Afterwards, we can remove these unnecessary static variables progressively and
> also gradually kill the shared const array.
Two things:
1. Please name the const array: sysctl_vals
2. What is missing from this patchset to completely kill sysctl_vals?
>
> Wen Yang (9):
> sysctl: support encoding values directly in the table entry
> kernel/sysctl-test: add some kunit test cases for min/max detection
> rxrpc: delete these unnecessary static variables n_65535, four,
> max_500, etc
> net: ipv6: delete these unnecessary static variables two_five_five and
> minus_one
> svcrdma: delete these unnecessary static variables min_ord, max_ord,
> etc
> sysctl: delete these unnecessary static variables i_zero and
> i_one_hundred
> epoll: delete these unnecessary static variables long_zero and
> long_max
> fs: inotify: delete these unnecessary static variables it_zero and
> it_int_max
> ucounts: delete these unnecessary static variables ue_zero and
> ue_int_max
>
> fs/eventpoll.c | 19 +-
> fs/notify/inotify/inotify_user.c | 49 +++--
> include/linux/sysctl.h | 108 ++++++++++-
> kernel/sysctl-test.c | 300 +++++++++++++++++++++++++++++++
> kernel/sysctl.c | 61 +++++--
> kernel/ucount.c | 8 +-
> lib/test_sysctl.c | 12 +-
> net/ipv6/addrconf.c | 15 +-
> net/rxrpc/sysctl.c | 169 ++++++++---------
> net/sunrpc/xprtrdma/svc_rdma.c | 21 +--
> 10 files changed, 571 insertions(+), 191 deletions(-)
>
> Cc: Eric W. Biederman <ebiederm@xmission.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: linux-kernel@vger.kernel.org
>
> --
> 2.25.1
>
--
Joel Granados
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 0/9] encode the values directly in the table entry
2024-03-21 15:21 ` Joel Granados
@ 2024-03-21 15:39 ` Wen Yang
0 siblings, 0 replies; 3+ messages in thread
From: Wen Yang @ 2024-03-21 15:39 UTC (permalink / raw)
To: Joel Granados
Cc: Eric W . Biederman, Luis Chamberlain, Kees Cook,
Christian Brauner, linux-kernel
On 2024/3/21 23:21, Joel Granados wrote:
> On Sat, Mar 09, 2024 at 06:31:17PM +0800, wenyang.linux@foxmail.com wrote:
>> From: Wen Yang <wenyang.linux@foxmail.com>
>>
>> The boundary check of multiple modules uses these static variables (such as
>> two_five_five, n_65535, ue_int_max, etc), and they are also not changed.
> This message is a bit cryptic. I had to do a fair amount of research to
> get what you meant here. Having the context in front is OK with me, but
> I would add a bit more information so the reader does not have to go to
> the code and grep for the variables that you mean. Something like this:
> "When using a sysctl proc_handler that requires a boundary check (like
> proce_dointvec_minmax) it is common to use a const variable like n_65535
> in net/rxrpc/sysctl.c or OTHER EXAMPLES...). This is suboptimal because
> YOUR REASONS HERE"
>
>
Thanks a lot for your very considerate input – this is highly
appreciated. We will revise the change message and send v3 later.
--
Best wishes,
Wen
>>
>> Eric points out: "by turning .extra1 and .extra2 into longs instead of
>> keeping them as pointers and needing constants to be pointed at somewhere
>> ... 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."
> I think it would be better to just link to the lore discussion.
>
>>
>> This patch series achieves direct encoding values in table entries and still
>> maintains compatibility with existing extra1/extra2 pointers.
>> Afterwards, we can remove these unnecessary static variables progressively and
>> also gradually kill the shared const array.
> Two things:
> 1. Please name the const array: sysctl_vals
> 2. What is missing from this patchset to completely kill sysctl_vals?
>
>>
>> Wen Yang (9):
>> sysctl: support encoding values directly in the table entry
>> kernel/sysctl-test: add some kunit test cases for min/max detection
>> rxrpc: delete these unnecessary static variables n_65535, four,
>> max_500, etc
>> net: ipv6: delete these unnecessary static variables two_five_five and
>> minus_one
>> svcrdma: delete these unnecessary static variables min_ord, max_ord,
>> etc
>> sysctl: delete these unnecessary static variables i_zero and
>> i_one_hundred
>> epoll: delete these unnecessary static variables long_zero and
>> long_max
>> fs: inotify: delete these unnecessary static variables it_zero and
>> it_int_max
>> ucounts: delete these unnecessary static variables ue_zero and
>> ue_int_max
>>
>> fs/eventpoll.c | 19 +-
>> fs/notify/inotify/inotify_user.c | 49 +++--
>> include/linux/sysctl.h | 108 ++++++++++-
>> kernel/sysctl-test.c | 300 +++++++++++++++++++++++++++++++
>> kernel/sysctl.c | 61 +++++--
>> kernel/ucount.c | 8 +-
>> lib/test_sysctl.c | 12 +-
>> net/ipv6/addrconf.c | 15 +-
>> net/rxrpc/sysctl.c | 169 ++++++++---------
>> net/sunrpc/xprtrdma/svc_rdma.c | 21 +--
>> 10 files changed, 571 insertions(+), 191 deletions(-)
>>
>> Cc: Eric W. Biederman <ebiederm@xmission.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: linux-kernel@vger.kernel.org
>>
>> --
>> 2.25.1
>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-03-21 15:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20240309103209eucas1p25311b485e321e701eeacbdb3e2ba8758@eucas1p2.samsung.com>
2024-03-09 10:31 ` [PATCH v2 0/9] encode the values directly in the table entry wenyang.linux
2024-03-21 15:21 ` Joel Granados
2024-03-21 15:39 ` Wen Yang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.