2024-02-25 04:06:38

by Wen Yang

[permalink] [raw]

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

---

From: Wen Yang <[email protected]>

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 <[email protected]>
Cc: Luis Chamberlain <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Joel Granados <[email protected]>
Cc: Christian Brauner <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
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, };
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, };
EXPORT_SYMBOL_GPL(sysctl_long_vals);

#if defined(CONFIG_SYSCTL)
--
2.25.1

2024-02-25 20:17:36

by Joel Granados

[permalink] [raw]

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

---

On Sun, Feb 25, 2024 at 12:05:31PM +0800, [email protected] wrote:
> From: Wen Yang <[email protected]>
>
> 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 <[email protected]>
> Cc: Luis Chamberlain <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Joel Granados <[email protected]>
> Cc: Christian Brauner <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: David Ahern <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: Jakub Kicinski <[email protected]>
> Cc: Paolo Abeni <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> 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.

> 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
>

--

Joel Granados

---

Attachments:

(No filename) (3.48 kB)
signature.asc (673.00 B)
Download all attachments

2024-02-26 15:21:51

by Wen Yang

[permalink] [raw]

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

---

On 2024/2/26 04:16, Joel Granados wrote:
> On Sun, Feb 25, 2024 at 12:05:31PM +0800, [email protected] wrote:
>> From: Wen Yang <[email protected]>
>>
>> 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 <[email protected]>
>> Cc: Luis Chamberlain <[email protected]>
>> Cc: Kees Cook <[email protected]>
>> Cc: Joel Granados <[email protected]>
>> Cc: Christian Brauner <[email protected]>
>> Cc: "David S. Miller" <[email protected]>
>> Cc: David Ahern <[email protected]>
>> Cc: Eric Dumazet <[email protected]>
>> Cc: Jakub Kicinski <[email protected]>
>> Cc: Paolo Abeni <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> ---
>> 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
>>
>