public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Wen Yang <wen.yang@linux.dev>
To: "Eric W . Biederman" <ebiederm@xmission.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	Joel Granados <j.granados@samsung.com>
Cc: Christian Brauner <brauner@kernel.org>,
	linux-kernel@vger.kernel.org, Wen Yang <wen.yang@linux.dev>,
	Dave Young <dyoung@redhat.com>
Subject: [PATCH v3 2/5] sysctl: support encoding values directly in the table entry
Date: Sun, 15 Sep 2024 10:08:28 +0800	[thread overview]
Message-ID: <95ca2bfb46a3aaef026623cd1a08c3c39366d5df.1726365007.git.wen.yang@linux.dev> (raw)
In-Reply-To: <cover.1726365007.git.wen.yang@linux.dev>

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 supports encoding values directly in table entries through the
following work:
- extra1/extra2 and min/max are placed in one union to ensure that the
  previous code is not broken, then we have time to gradually remove these
  unnecessary extra1/extra2;
- two bits were used to represent the information of the above union:
  SYSCTL_FLAG_MIN: 0, using extra1. 1, using min.
  SYSCTL_FLAG_MAX: 0, using extra2. 1, using max.
- since the proc file's mode field only uses 9 bits(777), we could use the
  additional two bits(S_ISUID and S_ISGID) to temporarily represent
  SYSCTL_FLAG_MIN and SYSCTL_FLAG_MAX.
- added some helper macros.

By introducing long min/max to replace void * extra1/extra2 in most cases,
unnecessary variables can be removed to save memory and avoid attacks.

Signed-off-by: Wen Yang <wen.yang@linux.dev>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Joel Granados <j.granados@samsung.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Dave Young <dyoung@redhat.com>
Cc: linux-kernel@vger.kernel.org
---
 fs/proc/proc_sysctl.c  |  8 +++--
 include/linux/sysctl.h | 71 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 67 insertions(+), 12 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 90c99eb1abf6..e88d1dca2a80 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -848,8 +848,11 @@ static int proc_sys_getattr(struct mnt_idmap *idmap,
 		return PTR_ERR(head);
 
 	generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
-	if (table)
+	if (table) {
 		stat->mode = (stat->mode & S_IFMT) | table->mode;
+		stat->mode &= ~SYSCTL_FLAG_MIN;
+		stat->mode &= ~SYSCTL_FLAG_MAX;
+	}
 
 	sysctl_head_finish(head);
 	return 0;
@@ -1163,7 +1166,8 @@ static int sysctl_check_table(const char *path, struct ctl_table_header *header)
 		if (!entry->proc_handler)
 			err |= sysctl_err(path, entry, "No proc_handler");
 
-		if ((entry->mode & (S_IRUGO|S_IWUGO)) != entry->mode)
+		if ((entry->mode & (S_IRUGO|S_IWUGO|SYSCTL_FLAG_MIN|SYSCTL_FLAG_MAX))
+		    != entry->mode)
 			err |= sysctl_err(path, entry, "bogus .mode 0%o",
 				entry->mode);
 	}
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 20e3914ec53f..8e27e8350ca8 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -28,6 +28,7 @@
 #include <linux/rbtree.h>
 #include <linux/uidgid.h>
 #include <uapi/linux/sysctl.h>
+#include <uapi/linux/stat.h>
 
 /* For the /proc/sys support */
 struct completion;
@@ -61,6 +62,37 @@ extern const int sysctl_vals[];
 
 extern const unsigned long sysctl_long_vals[];
 
+#define SYSCTL_NUM_ZERO                         (0L)
+#define SYSCTL_NUM_ONE                          (1L)
+#define SYSCTL_NUM_TWO                          (2L)
+#define SYSCTL_NUM_THREE                        (3L)
+#define SYSCTL_NUM_FOUR                         (4L)
+#define SYSCTL_NUM_FIVE                         (5L)
+#define SYSCTL_NUM_SIX                          (6L)
+#define SYSCTL_NUM_SEVEN                        (7L)
+#define SYSCTL_NUM_EIGHT                        (8L)
+#define SYSCTL_NUM_NINE                         (9L)
+#define SYSCTL_NUM_TEN                          (10L)
+#define SYSCTL_NUM_SIXTEEN                      (16L)
+#define SYSCTL_NUM_THIRTY_ONE                   (31L)
+#define SYSCTL_NUM_NEG_THIRTY_ONE               (-31L)
+#define SYSCTL_NUM_ONE_HUNDRED                  (100L)
+#define SYSCTL_NUM_TWO_HUNDRED                  (200L)
+#define SYSCTL_NUM_S8_MAX                       (127L)
+#define SYSCTL_NUM_U8_MAX                       (255L)
+#define SYSCTL_NUM_FIVE_HUNDRED                 (500L)
+#define SYSCTL_NUM_ONE_THOUSAND                 (1000L)
+#define SYSCTL_NUM_THREE_THOUSAND               (3000L)
+#define SYSCTL_NUM_16K                          (16 * 1024L)
+#define SYSCTL_NUM_16M                          (16 * 1024 * 1024L)
+#define SYSCTL_NUM_SEC_PER_HOUR                 (60 * 60L)
+#define SYSCTL_NUM_U16_MAX                      (65535L)
+#define SYSCTL_NUM_SEC_PER_DAY                  (24 * 60 * 60L)
+#define SYSCTL_NUM_MS_PER_DAY                   (24 * 60 * 60 * 1000L)
+#define SYSCTL_NUM_INT_MAX                      (INT_MAX)
+#define SYSCTL_NUM_NEG_ONE                      (-1)
+#define SYSCTL_NUM_LONG_MAX                     (LONG_MAX)
+
 typedef int proc_handler(const struct ctl_table *ctl, int write, void *buffer,
 		size_t *lenp, loff_t *ppos);
 
@@ -131,6 +163,9 @@ static inline void *proc_sys_poll_event(struct ctl_table_poll *poll)
 #define DEFINE_CTL_TABLE_POLL(name)					\
 	struct ctl_table_poll name = __CTL_TABLE_POLL_INITIALIZER(name)
 
+#define  SYSCTL_FLAG_MIN			S_ISUID
+#define  SYSCTL_FLAG_MAX			S_ISGID
+
 /* A sysctl table is an array of struct ctl_table: */
 struct ctl_table {
 	const char *procname;		/* Text ID for /proc/sys, or zero */
@@ -139,8 +174,16 @@ struct ctl_table {
 	umode_t mode;
 	proc_handler *proc_handler;	/* Callback for text formatting */
 	struct ctl_table_poll *poll;
-	void *extra1;
-	void *extra2;
+	union {
+		struct {
+			void *extra1;
+			void *extra2;
+		};
+		struct {
+			long min;
+			long max;
+		};
+	};
 } __randomize_layout;
 
 struct ctl_node {
@@ -214,42 +257,50 @@ struct ctl_table_root {
 
 static inline unsigned int sysctl_range_min_u8(const struct ctl_table *table)
 {
-	return (table->extra1) ? *(unsigned int *) table->extra1 : 0;
+	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
+	       (table->extra1) ? *(unsigned int *) table->extra1 : 0;
 }
 
 static inline unsigned int sysctl_range_max_u8(const struct ctl_table *table)
 {
-	return (table->extra2) ? *(unsigned int *) table->extra2 : U8_MAX;
+	return (table->mode & SYSCTL_FLAG_MAX) ? table->max :
+	       (table->extra2) ? *(unsigned int *) table->extra2 : U8_MAX;
 }
 
 static inline int sysctl_range_min_int(const struct ctl_table *table)
 {
-	return (table->extra1) ? *(int *) table->extra1 : INT_MIN;
+	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
+	       (table->extra1) ? *(int *) table->extra1 : INT_MIN;
 }
 
 static inline int sysctl_range_max_int(const struct ctl_table *table)
 {
-	return (table->extra2) ? *(int *) table->extra2 : INT_MAX;
+	return (table->mode & SYSCTL_FLAG_MAX) ? table->max :
+	       (table->extra2) ? *(int *) table->extra2 : INT_MAX;
 }
 
 static inline unsigned int sysctl_range_min_uint(const struct ctl_table *table)
 {
-	return (table->extra1) ? *(unsigned int *) table->extra1 : 0;
+	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
+	       (table->extra1) ? *(unsigned int *) table->extra1 : 0;
 }
 
 static inline unsigned int sysctl_range_max_uint(const struct ctl_table *table)
 {
-	return (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
+	return (table->mode & SYSCTL_FLAG_MAX) ? table->max :
+	       (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
 }
 
 static inline unsigned long sysctl_range_min_ulong(const struct ctl_table *table)
 {
-	return (table->extra1) ? *(unsigned long *) table->extra1 : 0;
+	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
+	       (table->extra1) ? *(unsigned long *) table->extra1 : 0;
 }
 
 static inline unsigned long sysctl_range_max_ulong(const struct ctl_table *table)
 {
-	return (table->extra2) ? *(unsigned long *) table->extra2 : ULONG_MAX;
+	return (table->mode & SYSCTL_FLAG_MAX) ? table->max :
+	       (table->extra2) ? *(unsigned long *) table->extra2 : ULONG_MAX;
 }
 
 #ifdef CONFIG_SYSCTL
-- 
2.25.1


  parent reply	other threads:[~2024-09-15  2:09 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-15  2:08 [PATCH v3 0/5] sysctl: encode the min/max values directly in the table entry Wen Yang
2024-09-15  2:08 ` [PATCH v3 1/5] sysctl: add helper functions to extract table->extra1/extra2 Wen Yang
2024-09-15  2:08 ` Wen Yang [this message]
2024-09-15  6:25   ` [PATCH v3 2/5] sysctl: support encoding values directly in the table entry Thomas Weißschuh
2024-09-21  8:19     ` Wen Yang
2024-09-15  2:08 ` [PATCH v3 3/5] sysctl: add KUnit test code to check for encoding min/max in table entries Wen Yang
2024-09-15  2:08 ` [PATCH v3 4/5] sysctl: delete mmap_rnd_bits_{min/max} and mmap_rnd_compat_bits_{min/max} to save 16 bytes Wen Yang
2024-09-15  2:08 ` [PATCH v3 5/5] sysctl: delete six_hundred_forty_kb to save 4 bytes Wen Yang
2024-09-15  6:08 ` [PATCH v3 0/5] sysctl: encode the min/max values directly in the table entry Thomas Weißschuh

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=95ca2bfb46a3aaef026623cd1a08c3c39366d5df.1726365007.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 \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox