Re: [PATCH 1/8] sysctl: introduce sysctl SYSCTL_U8_MAX and SYSCTL_LONG_S32_MAX

From: Wen Yang
Date: Mon Feb 26 2024 - 10:13:07 EST




On 2024/2/26 04:16, Joel Granados wrote:
On Sun, Feb 25, 2024 at 12:05:31PM +0800, wenyang.linux@xxxxxxxxxxx wrote:
From: Wen Yang <wenyang.linux@xxxxxxxxxxx>

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.
Therefore, add them to the shared sysctl_vals and sysctl_long_vals to avoid
duplication.

Also rearranged sysctl_vals and sysctl_long_vals in numerical order.

Signed-off-by: Wen Yang <wenyang.linux@xxxxxxxxxxx>
Cc: Luis Chamberlain <mcgrof@xxxxxxxxxx>
Cc: Kees Cook <keescook@xxxxxxxxxxxx>
Cc: Joel Granados <j.granados@xxxxxxxxxxx>
Cc: Christian Brauner <brauner@xxxxxxxxxx>
Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
Cc: David Ahern <dsahern@xxxxxxxxxx>
Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
Cc: Jakub Kicinski <kuba@xxxxxxxxxx>
Cc: Paolo Abeni <pabeni@xxxxxxxxxx>
Cc: netdev@xxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
---
include/linux/sysctl.h | 15 +++++++++------
kernel/sysctl.c | 4 ++--
2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index ee7d33b89e9e..b7a13e4c411c 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -45,19 +45,22 @@ struct ctl_dir;
#define SYSCTL_FOUR ((void *)&sysctl_vals[4])
#define SYSCTL_ONE_HUNDRED ((void *)&sysctl_vals[5])
#define SYSCTL_TWO_HUNDRED ((void *)&sysctl_vals[6])
-#define SYSCTL_ONE_THOUSAND ((void *)&sysctl_vals[7])
-#define SYSCTL_THREE_THOUSAND ((void *)&sysctl_vals[8])
-#define SYSCTL_INT_MAX ((void *)&sysctl_vals[9])
+#define SYSCTL_U8_MAX ((void *)&sysctl_vals[7])
+#define SYSCTL_ONE_THOUSAND ((void *)&sysctl_vals[8])
+#define SYSCTL_THREE_THOUSAND ((void *)&sysctl_vals[9])
+#define SYSCTL_U16_MAX ((void *)&sysctl_vals[10])
+#define SYSCTL_INT_MAX ((void *)&sysctl_vals[11])
+#define SYSCTL_NEG_ONE ((void *)&sysctl_vals[12])
/* this is needed for the proc_dointvec_minmax for [fs_]overflow UID and GID */
-#define SYSCTL_MAXOLDUID ((void *)&sysctl_vals[10])
-#define SYSCTL_NEG_ONE ((void *)&sysctl_vals[11])
+#define SYSCTL_MAXOLDUID SYSCTL_U16_MAX
extern const int sysctl_vals[];
#define SYSCTL_LONG_ZERO ((void *)&sysctl_long_vals[0])
#define SYSCTL_LONG_ONE ((void *)&sysctl_long_vals[1])
-#define SYSCTL_LONG_MAX ((void *)&sysctl_long_vals[2])
+#define SYSCTL_LONG_S32_MAX ((void *)&sysctl_long_vals[2])
+#define SYSCTL_LONG_MAX ((void *)&sysctl_long_vals[3])
extern const unsigned long sysctl_long_vals[];
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 157f7ce2942d..e1e00937cb29 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -82,10 +82,10 @@
#endif
/* shared constants to be used in various sysctls */
-const int sysctl_vals[] = { 0, 1, 2, 3, 4, 100, 200, 1000, 3000, INT_MAX, 65535, -1 };
+const int sysctl_vals[] = { 0, 1, 2, 3, 4, 100, 200, U8_MAX, 1000, 3000, 65535, INT_MAX, -1, };
Why do you insert the new values instead of just appending?
I figure that the patch would be much smaller if you appended.



Thanks.
We originally intended to rearrange this array in numerical order, but now it seems unnecessary.
We will follow your suggestions and send v2 later.

--
Best wishes,
Wen


EXPORT_SYMBOL(sysctl_vals);
-const unsigned long sysctl_long_vals[] = { 0, 1, LONG_MAX };
+const unsigned long sysctl_long_vals[] = { 0, 1, S32_MAX, LONG_MAX, };
Same here. Why insert instead of append?
EXPORT_SYMBOL_GPL(sysctl_long_vals);
#if defined(CONFIG_SYSCTL)
--
2.25.1