All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wen Yang <wen.yang@linux.dev>
To: Joel Granados <j.granados@samsung.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Kees Cook <keescook@chromium.org>
Cc: "Eric W . Biederman" <ebiederm@xmission.com>,
	"Dave Young" <dyoung@redhat.com>,
	"Christian Brauner" <brauner@kernel.org>,
	"Thomas Weißschuh" <thomas@t-8ch.de>,
	linux-kernel@vger.kernel.org, "Wen Yang" <wen.yang@linux.dev>
Subject: [PATCH v5 0/5] sysctl: encode the min/max values directly in the table entry
Date: Sun,  9 Feb 2025 23:58:08 +0800	[thread overview]
Message-ID: <cover.1739115369.git.wen.yang@linux.dev> (raw)

Many modules use these additional static/global variables (such as
two_five_five, n_65535, ue_int_max, etc.) in the boundary checking of
sysctl, and they are read-only and never changed.

Eric points out: 

- One of the things that happens and that is worth acknowledging is there
- is code that wraps proc_doulongvec_minmax and proc_dointvec_minmax.
- Having the minmax information separate from the data pointer makes that
- wrapping easier.

- Further the min/max information is typically separate from other kinds
- of data.  So even when not wrapped it is nice just to take a quick
- glance and see what the minimums and maximums are.

- My original suggest was that we change struct ctl_table from:

- > /* A sysctl table is an array of struct ctl_table: */
- > struct ctl_table {
- > 	const char *procname;		/* Text ID for /proc/sys */
- > 	void *data;
- > 	int maxlen;
- > 	umode_t mode;
- > 	proc_handler *proc_handler;	/* Callback for text formatting */
- > 	struct ctl_table_poll *poll;
- > 	void *extra1;
- > 	void *extra2;
- > } __randomize_layout;

- to:

- > /* A sysctl table is an array of struct ctl_table: */
- > struct ctl_table {
- > 	const char *procname;		/* Text ID for /proc/sys */
- > 	void *data;
- > 	int maxlen;
- > 	umode_t mode;
- > 	proc_handler *proc_handler;	/* Callback for text formatting */
- > 	struct ctl_table_poll *poll;
- >       unsigned long min;
- >       unsigned long max;
- > } __randomize_layout;

- That is just replace extra1 and extra2 with min and max members.  The
- members don't have any reason to be pointers.  Without being pointers
- the min/max functions can just use long values to cap either ints or
- longs, and there is no room for error.  The integer promotion rules
- will ensure that even negative values can be stored in unsigned long
- min and max values successfully.  Plus it is all bog standard C
- so there is nothing special to learn.

- There are a bunch of fiddly little details to transition from where we
- are today.  The most straightforward way I can see of making the
- transition is to add the min and max members.  Come up with replacements
- for proc_doulongvec_minmax and proc_dointvec_minmax that read the new
- min and max members.  Update all of the users.  Update the few users
- that use extra1 or extra2 for something besides min and max.  Then
- remove extra1 and extra2.  At the end it is simpler and requires the
- same or a little less space.

This patch series achieves direct encoding min/max values in table entries
and still maintains compatibility with existing extra1/extra2 pointers.
Afterwards, we can remove these unnecessary static variables progressively and
also finally kill the shared const array.

v4: https://lore.kernel.org/all/20241201140058.5653-1-wen.yang@linux.dev/
v3: https://lore.kernel.org/all/cover.1726365007.git.wen.yang@linux.dev/
v2: https://lore.kernel.org/all/tencent_143077FB953D8B549153BB07F54C5AA4870A@qq.com/
v1: https://lore.kernel.org/all/tencent_95D22FF919A42A99DA3C886B322CBD983905@qq.com/

and
https://lore.kernel.org/all/20250105152853.211037-1-wen.yang@linux.dev/

Wen Yang (5):
  sysctl: simplify the min/max boundary check
  sysctl: add helper functions to extract table->extra1/extra2
  sysctl: support encoding values directly in the table entry
  sysctl: add kunit test code to check the min/max encoding of sysctl
    table entries
  sysctl: delete six_hundred_forty_kb to save 4 bytes

 fs/proc/proc_sysctl.c  |  35 ++-
 include/linux/sysctl.h |  64 ++++-
 kernel/sysctl-test.c   | 581 +++++++++++++++++++++++++++++++++++++++++
 kernel/sysctl.c        | 211 ++++++---------
 4 files changed, 739 insertions(+), 152 deletions(-)

-- 
2.25.1


             reply	other threads:[~2025-02-09 15:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-09 15:58 Wen Yang [this message]
2025-02-09 15:58 ` [PATCH v5 1/5] sysctl: simplify the min/max boundary check Wen Yang
2025-02-09 15:58 ` [PATCH v5 2/5] sysctl: add helper functions to extract table->extra1/extra2 Wen Yang
2025-02-09 15:58 ` [PATCH v5 3/5] sysctl: support encoding values directly in the table entry Wen Yang
2025-02-09 15:58 ` [PATCH v5 4/5] sysctl: add kunit test code to check the min/max encoding of sysctl table entries Wen Yang
2025-02-09 15:58 ` [PATCH v5 5/5] sysctl: delete six_hundred_forty_kb to save 4 bytes Wen Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1739115369.git.wen.yang@linux.dev \
    --to=wen.yang@linux.dev \
    --cc=brauner@kernel.org \
    --cc=dyoung@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=j.granados@samsung.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=thomas@t-8ch.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.