| Message ID | 20220722022416.137548-4-mfo@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce "sysctl:" module aliases | expand |
On Fri, Jul 22, 2022 at 11:24 AM Mauricio Faria de Oliveira <mfo@canonical.com> wrote: > > In order to expose a sysctl entry to modpost (file2alias.c, precisely) > we have to shadow 'struct ctl_table' in mod_devicetable.h, as scripts > should not access kernel headers or its types (see file2alias.c). > > The required field is '.procname' (basename of '/proc/sys/.../entry'). > > Since 'struct ctl_table' is annotated for structure randomization and > we need a known offset for '.procname' (remember, no kernel headers), > take it out of the randomized portion (as in, eg, 'struct task_struct'). > > Of course, add build-time checks for struct size and .procname offset > between both structs. (This has to be done on kernel side; for headers.) > > With that in place, use the regular macros in devicetable-offsets.c to > define SIZE_... and OFF_... macros for the shadow struct and the field > of interest. > > Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> > --- > fs/proc/proc_sysctl.c | 19 +++++++++++++++++++ > include/linux/mod_devicetable.h | 25 +++++++++++++++++++++++++ > include/linux/sysctl.h | 11 ++++++++++- > kernel/sysctl.c | 1 + > scripts/mod/devicetable-offsets.c | 3 +++ > 5 files changed, 58 insertions(+), 1 deletion(-) > > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c > index 021e83fe831f..ebbf8702387e 100644 > --- a/fs/proc/proc_sysctl.c > +++ b/fs/proc/proc_sysctl.c > @@ -19,6 +19,24 @@ > #include <linux/kmemleak.h> > #include "internal.h" > > +#ifdef CONFIG_MODULES > +#include <linux/mod_devicetable.h> > + > +static void check_struct_sysctl_device_id(void) > +{ > + /* > + * The shadow struct sysctl_device_id for file2alias.c needs > + * the same size of struct ctl_table and offset for procname. > + */ > + BUILD_BUG_ON(sizeof(struct sysctl_device_id) > + != sizeof(struct ctl_table)); > + BUILD_BUG_ON(offsetof(struct sysctl_device_id, procname) > + != offsetof(struct ctl_table, procname)); Nit: If you use static_assert(), you can remove check_struct_sysctl_device_id(). You can write static_assert() out of a function. > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index 223376959d29..15073621cfa8 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -2487,6 +2487,7 @@ int __init sysctl_init_bases(void) > > return 0; > } > + Noise. > #endif /* CONFIG_SYSCTL */ > /* > * No sense putting this after each symbol definition, twice, > diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c > index c0d3bcb99138..43b2549940d2 100644 > --- a/scripts/mod/devicetable-offsets.c > +++ b/scripts/mod/devicetable-offsets.c > @@ -262,5 +262,8 @@ int main(void) > DEVID(ishtp_device_id); > DEVID_FIELD(ishtp_device_id, guid); > > + DEVID(sysctl_device_id); > + DEVID_FIELD(sysctl_device_id, procname); > + > return 0; > } > -- > 2.25.1 >
On Tue, Jul 26, 2022 at 6:27 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Fri, Jul 22, 2022 at 11:24 AM Mauricio Faria de Oliveira > <mfo@canonical.com> wrote: > > > > In order to expose a sysctl entry to modpost (file2alias.c, precisely) > > we have to shadow 'struct ctl_table' in mod_devicetable.h, as scripts > > should not access kernel headers or its types (see file2alias.c). > > > > The required field is '.procname' (basename of '/proc/sys/.../entry'). > > > > Since 'struct ctl_table' is annotated for structure randomization and > > we need a known offset for '.procname' (remember, no kernel headers), > > take it out of the randomized portion (as in, eg, 'struct task_struct'). > > > > Of course, add build-time checks for struct size and .procname offset > > between both structs. (This has to be done on kernel side; for headers.) > > > > With that in place, use the regular macros in devicetable-offsets.c to > > define SIZE_... and OFF_... macros for the shadow struct and the field > > of interest. > > > > Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> > > --- > > fs/proc/proc_sysctl.c | 19 +++++++++++++++++++ > > include/linux/mod_devicetable.h | 25 +++++++++++++++++++++++++ > > include/linux/sysctl.h | 11 ++++++++++- > > kernel/sysctl.c | 1 + > > scripts/mod/devicetable-offsets.c | 3 +++ > > 5 files changed, 58 insertions(+), 1 deletion(-) > > > > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c > > index 021e83fe831f..ebbf8702387e 100644 > > --- a/fs/proc/proc_sysctl.c > > +++ b/fs/proc/proc_sysctl.c > > @@ -19,6 +19,24 @@ > > #include <linux/kmemleak.h> > > #include "internal.h" > > > > +#ifdef CONFIG_MODULES > > +#include <linux/mod_devicetable.h> > > + > > +static void check_struct_sysctl_device_id(void) > > +{ > > + /* > > + * The shadow struct sysctl_device_id for file2alias.c needs > > + * the same size of struct ctl_table and offset for procname. > > + */ > > + BUILD_BUG_ON(sizeof(struct sysctl_device_id) > > + != sizeof(struct ctl_table)); > > + BUILD_BUG_ON(offsetof(struct sysctl_device_id, procname) > > + != offsetof(struct ctl_table, procname)); > > > Nit: > > If you use static_assert(), you can remove > check_struct_sysctl_device_id(). > > > You can write static_assert() out of a function. That's a nice cleanup; thanks! > > > > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > > index 223376959d29..15073621cfa8 100644 > > --- a/kernel/sysctl.c > > +++ b/kernel/sysctl.c > > @@ -2487,6 +2487,7 @@ int __init sysctl_init_bases(void) > > > > return 0; > > } > > + > > > Noise. Fixed. > > > > > > #endif /* CONFIG_SYSCTL */ > > /* > > * No sense putting this after each symbol definition, twice, > > diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c > > index c0d3bcb99138..43b2549940d2 100644 > > --- a/scripts/mod/devicetable-offsets.c > > +++ b/scripts/mod/devicetable-offsets.c > > @@ -262,5 +262,8 @@ int main(void) > > DEVID(ishtp_device_id); > > DEVID_FIELD(ishtp_device_id, guid); > > > > + DEVID(sysctl_device_id); > > + DEVID_FIELD(sysctl_device_id, procname); > > + > > return 0; > > } > > -- > > 2.25.1 > > > > > -- > Best Regards > Masahiro Yamada -- Mauricio Faria de Oliveira
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 021e83fe831f..ebbf8702387e 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -19,6 +19,24 @@ #include <linux/kmemleak.h> #include "internal.h" +#ifdef CONFIG_MODULES +#include <linux/mod_devicetable.h> + +static void check_struct_sysctl_device_id(void) +{ + /* + * The shadow struct sysctl_device_id for file2alias.c needs + * the same size of struct ctl_table and offset for procname. + */ + BUILD_BUG_ON(sizeof(struct sysctl_device_id) + != sizeof(struct ctl_table)); + BUILD_BUG_ON(offsetof(struct sysctl_device_id, procname) + != offsetof(struct ctl_table, procname)); +} +#else +static void check_struct_sysctl_device_id(void) {} +#endif + #define list_for_each_table_entry(entry, table) \ for ((entry) = (table); (entry)->procname; (entry)++) @@ -1779,6 +1797,7 @@ int __init proc_sys_init(void) proc_sys_root->proc_dir_ops = &proc_sys_dir_file_operations; proc_sys_root->nlink = 0; + check_struct_sysctl_device_id(); return sysctl_init_bases(); } diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index 549590e9c644..9cee024d8f2f 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -20,6 +20,31 @@ enum { PCI_ID_F_VFIO_DRIVER_OVERRIDE = 1, }; +/* + * "Device" table entry for a sysctl file (shadow of struct ctl_table). + * + * Only the procname field is reliable (known offset); all other fields + * are in the randomized portion of struct ctl_table, do NOT use them. + */ +struct sysctl_device_id { + + /* This must be the first field (shadowed from struct ctl_table). */ + const char *procname; + + /* Here begins the randomizable portion of struct ctl_table. */ + + void *data; + int maxlen; + unsigned short mode; // umode_t in <linux/types.h> + void *child; + void *proc_handler; + void *poll; + void *extra1; + void *extra2; + + /* Here ends the randomizable portion of struct ctl_table. */ +}; + /** * struct pci_device_id - PCI device ID structure * @vendor: Vendor ID to match (or PCI_ANY_ID) diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index 780690dc08cd..676112fde5ff 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -133,7 +133,13 @@ static inline void *proc_sys_poll_event(struct ctl_table_poll *poll) /* A sysctl table is an array of struct ctl_table: */ struct ctl_table { + + /* This must be the first field (shadowed to struct sysctl_device_id) */ const char *procname; /* Text ID for /proc/sys, or zero */ + + /* This begins the randomizable portion of the struct. */ + randomized_struct_fields_start + void *data; int maxlen; umode_t mode; @@ -142,7 +148,10 @@ struct ctl_table { struct ctl_table_poll *poll; void *extra1; void *extra2; -} __randomize_layout; + + /* New fields go above here, so they are in the randomized portion. */ + randomized_struct_fields_end +}; struct ctl_node { struct rb_node node; diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 223376959d29..15073621cfa8 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2487,6 +2487,7 @@ int __init sysctl_init_bases(void) return 0; } + #endif /* CONFIG_SYSCTL */ /* * No sense putting this after each symbol definition, twice, diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c index c0d3bcb99138..43b2549940d2 100644 --- a/scripts/mod/devicetable-offsets.c +++ b/scripts/mod/devicetable-offsets.c @@ -262,5 +262,8 @@ int main(void) DEVID(ishtp_device_id); DEVID_FIELD(ishtp_device_id, guid); + DEVID(sysctl_device_id); + DEVID_FIELD(sysctl_device_id, procname); + return 0; }
In order to expose a sysctl entry to modpost (file2alias.c, precisely) we have to shadow 'struct ctl_table' in mod_devicetable.h, as scripts should not access kernel headers or its types (see file2alias.c). The required field is '.procname' (basename of '/proc/sys/.../entry'). Since 'struct ctl_table' is annotated for structure randomization and we need a known offset for '.procname' (remember, no kernel headers), take it out of the randomized portion (as in, eg, 'struct task_struct'). Of course, add build-time checks for struct size and .procname offset between both structs. (This has to be done on kernel side; for headers.) With that in place, use the regular macros in devicetable-offsets.c to define SIZE_... and OFF_... macros for the shadow struct and the field of interest. Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> --- fs/proc/proc_sysctl.c | 19 +++++++++++++++++++ include/linux/mod_devicetable.h | 25 +++++++++++++++++++++++++ include/linux/sysctl.h | 11 ++++++++++- kernel/sysctl.c | 1 + scripts/mod/devicetable-offsets.c | 3 +++ 5 files changed, 58 insertions(+), 1 deletion(-)