| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240319165049.d7m4jvwb4qyjtjhq@joelS2.panther.com>
Date: Tue, 19 Mar 2024 17:50:49 +0100
From: Joel Granados <j.granados@...sung.com>
To: <wenyang.linux@...mail.com>
CC: "Eric W . Biederman" <ebiederm@...ssion.com>, Luis Chamberlain
<mcgrof@...nel.org>, Kees Cook <keescook@...omium.org>, Christian Brauner
<brauner@...nel.org>, Dave Young <dyoung@...hat.com>, Iurii Zaikin
<yzaikin@...gle.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH v2 1/9] sysctl: support encoding values directly
in the table entry
This is a resend on the one 10 days ago. Right?
I have it in my todo list and will get to it eventually. Please stop
resending it if there is no change.
Thx
On Tue, Mar 19, 2024 at 11:57:42PM +0800, wenyang.linux@...mail.com wrote:
> From: Wen Yang <wenyang.linux@...mail.com>
>
> Eric pointed 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 remove unnecessary
> extra1/extra2 progressively;
> - since type only has two states, use one bit to represent it;
> - two bits were used to represent the information of the above union( 0:
> using extra1/extra2, 1: using min, 2: using max, 3: using both min/max);
> - added some helper macros.
>
> Suggested-by: Eric W. Biederman <ebiederm@...ssion.com>
> Signed-off-by: Wen Yang <wenyang.linux@...mail.com>
> Cc: Luis Chamberlain <mcgrof@...nel.org>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Joel Granados <j.granados@...sung.com>
> Cc: Eric W. Biederman <ebiederm@...ssion.com>
> Cc: Christian Brauner <brauner@...nel.org>
> Cc: Iurii Zaikin <yzaikin@...gle.com>
> Cc: linux-kernel@...r.kernel.org
> ---
> include/linux/sysctl.h | 108 ++++++++++++++++++++++++++++++++++++++---
> kernel/sysctl.c | 61 +++++++++++++++++------
> 2 files changed, 148 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index ee7d33b89e9e..1ba980219e40 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -61,6 +61,25 @@ extern const int sysctl_vals[];
>
> extern const unsigned long sysctl_long_vals[];
>
> +#define SYSCTL_NUMERIC_NEG_ONE ((long)-1)
> +#define SYSCTL_NUMERIC_ZERO (0L)
> +#define SYSCTL_NUMERIC_ONE (1L)
> +#define SYSCTL_NUMERIC_TWO (2L)
> +#define SYSCTL_NUMERIC_THREE (3L)
> +#define SYSCTL_NUMERIC_FOUR (4L)
> +#define SYSCTL_NUMERIC_ONE_HUNDRED (100L)
> +#define SYSCTL_NUMERIC_TWO_HUNDRED (200L)
> +#define SYSCTL_NUMERIC_THREE_HUNDRED (300L)
> +#define SYSCTL_NUMERIC_FIVE_HUNDRED (500L)
> +#define SYSCTL_NUMERIC_ONE_THOUSAND (1000L)
> +#define SYSCTL_NUMERIC_TWO_THOUSAND (2000L)
> +#define SYSCTL_NUMERIC_THREE_THOUSAND (3000L)
> +#define SYSCTL_NUMERIC_16K (16384L)
> +#define SYSCTL_NUMERIC_U8_MAX ((long)U8_MAX)
> +#define SYSCTL_NUMERIC_U16_MAX ((long)U16_MAX)
> +#define SYSCTL_NUMERIC_INT_MAX ((long)INT_MAX)
> +#define SYSCTL_NUMERIC_LONG_MAX (LONG_MAX)
> +
> typedef int proc_handler(struct ctl_table *ctl, int write, void *buffer,
> size_t *lenp, loff_t *ppos);
>
> @@ -131,6 +150,18 @@ 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)
>
> +enum {
> + SYSCTL_TABLE_TYPE_DEFAULT,
> + SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY
> +};
> +
> +enum {
> + SYSCTL_TABLE_EXTRA_PTR,
> + SYSCTL_TABLE_EXTRA_LONG_INIT_MIN,
> + SYSCTL_TABLE_EXTRA_LONG_INIT_MAX,
> + SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX
> +};
> +
> /* A sysctl table is an array of struct ctl_table: */
> struct ctl_table {
> const char *procname; /* Text ID for /proc/sys, or zero */
> @@ -138,20 +169,39 @@ struct ctl_table {
> int maxlen;
> umode_t mode;
> /**
> - * enum type - Enumeration to differentiate between ctl target types
> + * type - Indicates to differentiate between ctl target types
> * @SYSCTL_TABLE_TYPE_DEFAULT: ctl target with no special considerations
> * @SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY: Used to identify a permanently
> * empty directory target to serve
> * as mount point.
> */
> - enum {
> - SYSCTL_TABLE_TYPE_DEFAULT,
> - SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY
> - } type;
> + u8 type:1;
> +
> + /**
> + * extra_flags
> + * @SYSCTL_TABLE_EXTRA_PTR: flag indicating that this uses extra1/extra2.
> + * @SYSCTL_TABLE_EXTRA_LONG_INIT_MIN: flag indicating that this uses min/max
> + and min has been initialized.
> + * @SYSCTL_TABLE_EXTRA_LONG_INIT_MAX: flag indicating that this uses min/max
> + and max has been initialized.
> + * @SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX: flag indicating that this uses min/max
> + and both have been initialized.
> + *
> + */
> + u8 extra_flags:2;
> + union {
> + struct {
> + void *extra1;
> + void *extra2;
> + };
> + struct {
> + long min;
> + long max;
> + };
> + };
> +
> proc_handler *proc_handler; /* Callback for text formatting */
> struct ctl_table_poll *poll;
> - void *extra1;
> - void *extra2;
> } __randomize_layout;
>
> struct ctl_node {
> @@ -213,6 +263,50 @@ struct ctl_table_root {
> #define register_sysctl(path, table) \
> register_sysctl_sz(path, table, ARRAY_SIZE(table))
>
> +#define CTL_TABLE_ENTRY(_name, _data, _len, _mode, _func) \
> + { \
> + .procname = _name, \
> + .data = _data, \
> + .maxlen = _len, \
> + .mode = _mode, \
> + .proc_handler = _func, \
> + .extra_flags = SYSCTL_TABLE_EXTRA_PTR, \
> + }
> +
> +#define CTL_TABLE_ENTRY_MIN(_name, _data, _len, _mode, _func, _min) \
> + { \
> + .procname = _name, \
> + .data = _data, \
> + .maxlen = _len, \
> + .mode = _mode, \
> + .proc_handler = _func, \
> + .min = _min, \
> + .extra_flags = SYSCTL_TABLE_EXTRA_LONG_INIT_MIN, \
> + }
> +
> +#define CTL_TABLE_ENTRY_MAX(_name, _data, _len, _mode, _func, _max) \
> + { \
> + .procname = _name, \
> + .data = _data, \
> + .maxlen = _len, \
> + .mode = _mode, \
> + .proc_handler = _func, \
> + .max = _max, \
> + .extra_flags = SYSCTL_TABLE_EXTRA_LONG_INIT_MAX, \
> + }
> +
> +#define CTL_TABLE_ENTRY_MINMAX(_name, _data, _len, _mode, _func, _min, _max) \
> + { \
> + .procname = _name, \
> + .data = _data, \
> + .maxlen = _len, \
> + .mode = _mode, \
> + .proc_handler = _func, \
> + .min = _min, \
> + .max = _max, \
> + .extra_flags = SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX, \
> + }
> +
> #ifdef CONFIG_SYSCTL
>
> void proc_sys_poll_notify(struct ctl_table_poll *poll);
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 157f7ce2942d..144c441236ab 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -822,6 +822,20 @@ struct do_proc_dointvec_minmax_conv_param {
> int *max;
> };
>
> +static void do_int_conv_param_init(const struct ctl_table *table,
> + struct do_proc_dointvec_minmax_conv_param *param)
> +{
> + if (table->extra_flags == SYSCTL_TABLE_EXTRA_PTR) {
> + param->min = table->extra1;
> + param->max = table->extra2;
> + } else {
> + param->min = (table->extra_flags & SYSCTL_TABLE_EXTRA_LONG_INIT_MIN) ?
> + (int *)&table->min : NULL;
> + param->max = (table->extra_flags & SYSCTL_TABLE_EXTRA_LONG_INIT_MAX) ?
> + (int *)&table->max : NULL;
> + }
> +}
> +
> static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
> int *valp,
> int write, void *data)
> @@ -867,10 +881,9 @@ static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
> int proc_dointvec_minmax(struct ctl_table *table, int write,
> void *buffer, size_t *lenp, loff_t *ppos)
> {
> - struct do_proc_dointvec_minmax_conv_param param = {
> - .min = (int *) table->extra1,
> - .max = (int *) table->extra2,
> - };
> + struct do_proc_dointvec_minmax_conv_param param;
> +
> + do_int_conv_param_init(table, ¶m);
> return do_proc_dointvec(table, write, buffer, lenp, ppos,
> do_proc_dointvec_minmax_conv, ¶m);
> }
> @@ -889,6 +902,20 @@ struct do_proc_douintvec_minmax_conv_param {
> unsigned int *max;
> };
>
> +static void do_uint_conv_param_init(const struct ctl_table *table,
> + struct do_proc_douintvec_minmax_conv_param *param)
> +{
> + if (table->extra_flags == SYSCTL_TABLE_EXTRA_PTR) {
> + param->min = table->extra1;
> + param->max = table->extra2;
> + } else {
> + param->min = (table->extra_flags & SYSCTL_TABLE_EXTRA_LONG_INIT_MIN) ?
> + (unsigned int *)&table->min : NULL;
> + param->max = (table->extra_flags & SYSCTL_TABLE_EXTRA_LONG_INIT_MAX) ?
> + (unsigned int *)&table->max : NULL;
> + }
> +}
> +
> static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
> unsigned int *valp,
> int write, void *data)
> @@ -936,10 +963,9 @@ static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
> int proc_douintvec_minmax(struct ctl_table *table, int write,
> void *buffer, size_t *lenp, loff_t *ppos)
> {
> - struct do_proc_douintvec_minmax_conv_param param = {
> - .min = (unsigned int *) table->extra1,
> - .max = (unsigned int *) table->extra2,
> - };
> + struct do_proc_douintvec_minmax_conv_param param;
> +
> + do_uint_conv_param_init(table, ¶m);
> return do_proc_douintvec(table, write, buffer, lenp, ppos,
> do_proc_douintvec_minmax_conv, ¶m);
> }
> @@ -1038,8 +1064,16 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table,
> }
>
> i = data;
> - min = table->extra1;
> - max = table->extra2;
> + if (table->extra_flags == SYSCTL_TABLE_EXTRA_PTR) {
> + min = table->extra1;
> + max = table->extra2;
> + } else {
> + min = (table->extra_flags & SYSCTL_TABLE_EXTRA_LONG_INIT_MIN) ?
> + &table->min : NULL;
> + max = (table->extra_flags & SYSCTL_TABLE_EXTRA_LONG_INIT_MAX) ?
> + &table->max : NULL;
> + }
> +
> vleft = table->maxlen / sizeof(unsigned long);
> left = *lenp;
>
> @@ -1274,10 +1308,9 @@ int proc_dointvec_jiffies(struct ctl_table *table, int write,
> int proc_dointvec_ms_jiffies_minmax(struct ctl_table *table, int write,
> void *buffer, size_t *lenp, loff_t *ppos)
> {
> - struct do_proc_dointvec_minmax_conv_param param = {
> - .min = (int *) table->extra1,
> - .max = (int *) table->extra2,
> - };
> + struct do_proc_dointvec_minmax_conv_param param;
> +
> + do_int_conv_param_init(table, ¶m);
> return do_proc_dointvec(table, write, buffer, lenp, ppos,
> do_proc_dointvec_ms_jiffies_minmax_conv, ¶m);
> }
> --
> 2.25.1
>
--
Joel Granados
Download attachment "signature.asc" of type "application/pgp-signature" (660 bytes)
Powered by blists - more mailing lists