* [PATCH v2 0/1] Add ro_after_init support for modules @ 2016-07-25 9:25 Jessica Yu 2016-07-25 9:25 ` [PATCH v2 1/1] modules: add ro_after_init support Jessica Yu 0 siblings, 1 reply; 9+ messages in thread From: Jessica Yu @ 2016-07-25 9:25 UTC (permalink / raw) To: Rusty Russell, Kees Cook Cc: linux-api, linux-kernel, live-patching, Jessica Yu Hi, This patch adds ro_after_init support for modules by adding an additional page-aligned section in the module layout. This new ro_after_init section sits between rodata and writable data. So, the new module layout looks like: [text] [rodata] [ro_after_init] [writable data] RO after init data remains RW during init and RO protection is enabled separately after module init runs. Did some light testing with lkdtm compiled as a module, verified that ro_after_init data is writable during init, and that it oopsed after attempted writes after init. Also tested livepatch (which uses module_{enable,disable}_ro for its own purposes) to make sure nothing broke. More testing is appreciated :-) Some remarks on the implementation: * A new SHF_RO_AFTER_INIT flag is introduced in elf.h to make identification of .data..ro_after_init sections and the work of layout_sections() easier. Its chosen value is within the SHF_MASKOS range. * If a module doesn't have a ro_after_init section, then core_layout.ro_after_init_size just takes the value of core_layout.ro_size, and frob_ro_after_init() should do nothing. Based on linux-next. v1 here: http://lkml.kernel.org/g/1465863198-15947-1-git-send-email-jeyu@redhat.com v2: - Add a bool after_init parameter to module_enable_ro(), it's much clearer than checking module->state. - Since the function signature for module_enable_ro() has changed, livepatch needs to slightly adjust its call to module_enable_ro() Jessica Yu (1): modules: add ro_after_init support include/linux/module.h | 6 +++-- include/uapi/linux/elf.h | 1 + kernel/livepatch/core.c | 2 +- kernel/module.c | 66 +++++++++++++++++++++++++++++++++++++++--------- 4 files changed, 60 insertions(+), 15 deletions(-) -- 2.5.5 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/1] modules: add ro_after_init support 2016-07-25 9:25 [PATCH v2 0/1] Add ro_after_init support for modules Jessica Yu @ 2016-07-25 9:25 ` Jessica Yu 2016-07-25 10:01 ` Jessica Yu 2016-07-25 18:04 ` [PATCH v2 1/1] " Kees Cook 0 siblings, 2 replies; 9+ messages in thread From: Jessica Yu @ 2016-07-25 9:25 UTC (permalink / raw) To: Rusty Russell, Kees Cook Cc: linux-api, linux-kernel, live-patching, Jessica Yu Add ro_after_init support for modules by adding a new page-aligned section in the module layout (after rodata) for ro_after_init data and enabling RO protection for that section after module init runs. Signed-off-by: Jessica Yu <jeyu@redhat.com> --- include/linux/module.h | 6 +++-- include/uapi/linux/elf.h | 1 + kernel/livepatch/core.c | 2 +- kernel/module.c | 66 +++++++++++++++++++++++++++++++++++++++--------- 4 files changed, 60 insertions(+), 15 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index f777164..5255c2f 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -311,6 +311,8 @@ struct module_layout { unsigned int text_size; /* Size of RO section of the module (text+rodata) */ unsigned int ro_size; + /* Size of RO after init section */ + unsigned int ro_after_init_size; #ifdef CONFIG_MODULES_TREE_LOOKUP struct mod_tree_node mtn; @@ -788,12 +790,12 @@ extern int module_sysfs_initialized; #ifdef CONFIG_DEBUG_SET_MODULE_RONX extern void set_all_modules_text_rw(void); extern void set_all_modules_text_ro(void); -extern void module_enable_ro(const struct module *mod); +extern void module_enable_ro(const struct module *mod, bool after_init); extern void module_disable_ro(const struct module *mod); #else static inline void set_all_modules_text_rw(void) { } static inline void set_all_modules_text_ro(void) { } -static inline void module_enable_ro(const struct module *mod) { } +static inline void module_enable_ro(const struct module *mod, bool after_init) { } static inline void module_disable_ro(const struct module *mod) { } #endif diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h index cb4a72f..70b172ba 100644 --- a/include/uapi/linux/elf.h +++ b/include/uapi/linux/elf.h @@ -286,6 +286,7 @@ typedef struct elf64_phdr { #define SHF_ALLOC 0x2 #define SHF_EXECINSTR 0x4 #define SHF_RELA_LIVEPATCH 0x00100000 +#define SHF_RO_AFTER_INIT 0x00200000 #define SHF_MASKPROC 0xf0000000 /* special section indexes */ diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 5c2bc10..8bbe507 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -309,7 +309,7 @@ static int klp_write_object_relocations(struct module *pmod, break; } - module_enable_ro(pmod); + module_enable_ro(pmod, true); return ret; } diff --git a/kernel/module.c b/kernel/module.c index e70a2fa..e697144 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1858,10 +1858,11 @@ static void mod_sysfs_teardown(struct module *mod) * from modification and any data from execution. * * General layout of module is: - * [text] [read-only-data] [writable data] - * text_size -----^ ^ ^ - * ro_size ------------------------| | - * size -------------------------------------------| + * [text] [read-only-data] [ro-after-init] [writable data] + * text_size -----^ ^ ^ ^ + * ro_size ------------------------| | | + * ro_after_init_size -----------------------------| | + * size -----------------------------------------------------------| * * These values are always page-aligned (as is base) */ @@ -1884,14 +1885,24 @@ static void frob_rodata(const struct module_layout *layout, (layout->ro_size - layout->text_size) >> PAGE_SHIFT); } +static void frob_ro_after_init(const struct module_layout *layout, + int (*set_memory)(unsigned long start, int num_pages)) +{ + BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); + BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); + set_memory((unsigned long)layout->base + layout->ro_size, + (layout->ro_after_init_size - layout->ro_size) >> PAGE_SHIFT); +} + static void frob_writable_data(const struct module_layout *layout, int (*set_memory)(unsigned long start, int num_pages)) { BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); - BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); BUG_ON((unsigned long)layout->size & (PAGE_SIZE-1)); - set_memory((unsigned long)layout->base + layout->ro_size, - (layout->size - layout->ro_size) >> PAGE_SHIFT); + set_memory((unsigned long)layout->base + layout->ro_after_init_size, + (layout->size - layout->ro_after_init_size) >> PAGE_SHIFT); } /* livepatching wants to disable read-only so it can frob module. */ @@ -1899,21 +1910,26 @@ void module_disable_ro(const struct module *mod) { frob_text(&mod->core_layout, set_memory_rw); frob_rodata(&mod->core_layout, set_memory_rw); + frob_ro_after_init(&mod->core_layout, set_memory_rw); frob_text(&mod->init_layout, set_memory_rw); frob_rodata(&mod->init_layout, set_memory_rw); } -void module_enable_ro(const struct module *mod) +void module_enable_ro(const struct module *mod, bool after_init) { frob_text(&mod->core_layout, set_memory_ro); frob_rodata(&mod->core_layout, set_memory_ro); frob_text(&mod->init_layout, set_memory_ro); frob_rodata(&mod->init_layout, set_memory_ro); + + if (after_init) + frob_ro_after_init(&mod->core_layout, set_memory_ro); } static void module_enable_nx(const struct module *mod) { frob_rodata(&mod->core_layout, set_memory_nx); + frob_ro_after_init(&mod->core_layout, set_memory_nx); frob_writable_data(&mod->core_layout, set_memory_nx); frob_rodata(&mod->init_layout, set_memory_nx); frob_writable_data(&mod->init_layout, set_memory_nx); @@ -1922,6 +1938,7 @@ static void module_enable_nx(const struct module *mod) static void module_disable_nx(const struct module *mod) { frob_rodata(&mod->core_layout, set_memory_x); + frob_ro_after_init(&mod->core_layout, set_memory_x); frob_writable_data(&mod->core_layout, set_memory_x); frob_rodata(&mod->init_layout, set_memory_x); frob_writable_data(&mod->init_layout, set_memory_x); @@ -1964,6 +1981,8 @@ static void disable_ro_nx(const struct module_layout *layout) frob_text(layout, set_memory_rw); frob_rodata(layout, set_memory_rw); frob_rodata(layout, set_memory_x); + frob_ro_after_init(layout, set_memory_rw); + frob_ro_after_init(layout, set_memory_x); frob_writable_data(layout, set_memory_x); } @@ -2306,6 +2325,7 @@ static void layout_sections(struct module *mod, struct load_info *info) * finder in the two loops below */ { SHF_EXECINSTR | SHF_ALLOC, ARCH_SHF_SMALL }, { SHF_ALLOC, SHF_WRITE | ARCH_SHF_SMALL }, + { SHF_RO_AFTER_INIT | SHF_ALLOC, ARCH_SHF_SMALL }, { SHF_WRITE | SHF_ALLOC, ARCH_SHF_SMALL }, { ARCH_SHF_SMALL | SHF_ALLOC, 0 } }; @@ -2337,7 +2357,11 @@ static void layout_sections(struct module *mod, struct load_info *info) mod->core_layout.size = debug_align(mod->core_layout.size); mod->core_layout.ro_size = mod->core_layout.size; break; - case 3: /* whole core */ + case 2: /* RO after init */ + mod->core_layout.size = debug_align(mod->core_layout.size); + mod->core_layout.ro_after_init_size = mod->core_layout.size; + break; + case 4: /* whole core */ mod->core_layout.size = debug_align(mod->core_layout.size); break; } @@ -2367,7 +2391,14 @@ static void layout_sections(struct module *mod, struct load_info *info) mod->init_layout.size = debug_align(mod->init_layout.size); mod->init_layout.ro_size = mod->init_layout.size; break; - case 3: /* whole init */ + case 2: + /* + * RO after init doesn't apply to init_layout (only + * core_layout), so it just takes the value of ro_size. + */ + mod->init_layout.ro_after_init_size = mod->init_layout.ro_size; + break; + case 4: /* whole init */ mod->init_layout.size = debug_align(mod->init_layout.size); break; } @@ -3173,6 +3204,7 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) { /* Module within temporary copy. */ struct module *mod; + unsigned int ndx; int err; mod = setup_load_info(info, flags); @@ -3192,6 +3224,15 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) /* We will do a special allocation for per-cpu sections later. */ info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC; + /* + * Mark ro_after_init section with SHF_RO_AFTER_INIT so that + * layout_sections() can put it in the right place. + * Note: ro_after_init sections also have SHF_{WRITE,ALLOC} set. + */ + ndx = find_sec(info, ".data..ro_after_init"); + if (ndx) + info->sechdrs[ndx].sh_flags |= SHF_RO_AFTER_INIT; + /* Determine total sizes, and put offsets in sh_entsize. For now this is done generically; there doesn't appear to be any special cases for the architectures. */ @@ -3358,12 +3399,14 @@ static noinline int do_init_module(struct module *mod) /* Switch to core kallsyms now init is done: kallsyms may be walking! */ rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms); #endif + module_enable_ro(mod, true); mod_tree_remove_init(mod); disable_ro_nx(&mod->init_layout); module_arch_freeing_init(mod); mod->init_layout.base = NULL; mod->init_layout.size = 0; mod->init_layout.ro_size = 0; + mod->init_layout.ro_after_init_size = 0; mod->init_layout.text_size = 0; /* * We want to free module_init, but be aware that kallsyms may be @@ -3455,8 +3498,7 @@ static int complete_formation(struct module *mod, struct load_info *info) /* This relies on module_mutex for list integrity. */ module_bug_finalize(info->hdr, info->sechdrs, mod); - /* Set RO and NX regions */ - module_enable_ro(mod); + module_enable_ro(mod, false); module_enable_nx(mod); /* Mark state as coming so strong_try_module_get() ignores us, -- 2.5.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: modules: add ro_after_init support 2016-07-25 9:25 ` [PATCH v2 1/1] modules: add ro_after_init support Jessica Yu @ 2016-07-25 10:01 ` Jessica Yu 2016-07-25 18:04 ` [PATCH v2 1/1] " Kees Cook 1 sibling, 0 replies; 9+ messages in thread From: Jessica Yu @ 2016-07-25 10:01 UTC (permalink / raw) To: Rusty Russell, Kees Cook; +Cc: linux-api, linux-kernel, live-patching +++ Jessica Yu [25/07/16 05:25 -0400]: >Add ro_after_init support for modules by adding a new page-aligned section >in the module layout (after rodata) for ro_after_init data and enabling RO >protection for that section after module init runs. > >Signed-off-by: Jessica Yu <jeyu@redhat.com> >--- > include/linux/module.h | 6 +++-- > include/uapi/linux/elf.h | 1 + > kernel/livepatch/core.c | 2 +- > kernel/module.c | 66 +++++++++++++++++++++++++++++++++++++++--------- > 4 files changed, 60 insertions(+), 15 deletions(-) > >diff --git a/include/linux/module.h b/include/linux/module.h >index f777164..5255c2f 100644 >--- a/include/linux/module.h >+++ b/include/linux/module.h >@@ -311,6 +311,8 @@ struct module_layout { > unsigned int text_size; > /* Size of RO section of the module (text+rodata) */ > unsigned int ro_size; >+ /* Size of RO after init section */ >+ unsigned int ro_after_init_size; > > #ifdef CONFIG_MODULES_TREE_LOOKUP > struct mod_tree_node mtn; >@@ -788,12 +790,12 @@ extern int module_sysfs_initialized; > #ifdef CONFIG_DEBUG_SET_MODULE_RONX > extern void set_all_modules_text_rw(void); > extern void set_all_modules_text_ro(void); >-extern void module_enable_ro(const struct module *mod); >+extern void module_enable_ro(const struct module *mod, bool after_init); > extern void module_disable_ro(const struct module *mod); > #else > static inline void set_all_modules_text_rw(void) { } > static inline void set_all_modules_text_ro(void) { } >-static inline void module_enable_ro(const struct module *mod) { } >+static inline void module_enable_ro(const struct module *mod, bool after_init) { } > static inline void module_disable_ro(const struct module *mod) { } > #endif > >diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h >index cb4a72f..70b172ba 100644 >--- a/include/uapi/linux/elf.h >+++ b/include/uapi/linux/elf.h >@@ -286,6 +286,7 @@ typedef struct elf64_phdr { > #define SHF_ALLOC 0x2 > #define SHF_EXECINSTR 0x4 > #define SHF_RELA_LIVEPATCH 0x00100000 >+#define SHF_RO_AFTER_INIT 0x00200000 > #define SHF_MASKPROC 0xf0000000 > > /* special section indexes */ >diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c >index 5c2bc10..8bbe507 100644 >--- a/kernel/livepatch/core.c >+++ b/kernel/livepatch/core.c >@@ -309,7 +309,7 @@ static int klp_write_object_relocations(struct module *pmod, > break; > } > >- module_enable_ro(pmod); >+ module_enable_ro(pmod, true); There is a slight quirk here in that klp_init_object_loaded() (which calls klp_write_object_relocations()) can be called either during patch module init (during patch registration) or after init (e.g., when a previously unloaded to-be-patched module is loaded). AFAIK patch modules themselves don't use .data..ro_after_init sections, so it's probably fine to set after_init to be true here for now. But I still need to think some more about the case where we try to patch data from another module marked __ro_after_init. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/1] modules: add ro_after_init support 2016-07-25 9:25 ` [PATCH v2 1/1] modules: add ro_after_init support Jessica Yu 2016-07-25 10:01 ` Jessica Yu @ 2016-07-25 18:04 ` Kees Cook 1 sibling, 0 replies; 9+ messages in thread From: Kees Cook @ 2016-07-25 18:04 UTC (permalink / raw) To: Jessica Yu; +Cc: Rusty Russell, Linux API, LKML, live-patching On Mon, Jul 25, 2016 at 2:25 AM, Jessica Yu <jeyu@redhat.com> wrote: > Add ro_after_init support for modules by adding a new page-aligned section > in the module layout (after rodata) for ro_after_init data and enabling RO > protection for that section after module init runs. > > Signed-off-by: Jessica Yu <jeyu@redhat.com> Acked-by: Kees Cook <keescook@chromium.org> Thanks! -Kees > --- > include/linux/module.h | 6 +++-- > include/uapi/linux/elf.h | 1 + > kernel/livepatch/core.c | 2 +- > kernel/module.c | 66 +++++++++++++++++++++++++++++++++++++++--------- > 4 files changed, 60 insertions(+), 15 deletions(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index f777164..5255c2f 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -311,6 +311,8 @@ struct module_layout { > unsigned int text_size; > /* Size of RO section of the module (text+rodata) */ > unsigned int ro_size; > + /* Size of RO after init section */ > + unsigned int ro_after_init_size; > > #ifdef CONFIG_MODULES_TREE_LOOKUP > struct mod_tree_node mtn; > @@ -788,12 +790,12 @@ extern int module_sysfs_initialized; > #ifdef CONFIG_DEBUG_SET_MODULE_RONX > extern void set_all_modules_text_rw(void); > extern void set_all_modules_text_ro(void); > -extern void module_enable_ro(const struct module *mod); > +extern void module_enable_ro(const struct module *mod, bool after_init); > extern void module_disable_ro(const struct module *mod); > #else > static inline void set_all_modules_text_rw(void) { } > static inline void set_all_modules_text_ro(void) { } > -static inline void module_enable_ro(const struct module *mod) { } > +static inline void module_enable_ro(const struct module *mod, bool after_init) { } > static inline void module_disable_ro(const struct module *mod) { } > #endif > > diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h > index cb4a72f..70b172ba 100644 > --- a/include/uapi/linux/elf.h > +++ b/include/uapi/linux/elf.h > @@ -286,6 +286,7 @@ typedef struct elf64_phdr { > #define SHF_ALLOC 0x2 > #define SHF_EXECINSTR 0x4 > #define SHF_RELA_LIVEPATCH 0x00100000 > +#define SHF_RO_AFTER_INIT 0x00200000 > #define SHF_MASKPROC 0xf0000000 > > /* special section indexes */ > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index 5c2bc10..8bbe507 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -309,7 +309,7 @@ static int klp_write_object_relocations(struct module *pmod, > break; > } > > - module_enable_ro(pmod); > + module_enable_ro(pmod, true); > return ret; > } > > diff --git a/kernel/module.c b/kernel/module.c > index e70a2fa..e697144 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -1858,10 +1858,11 @@ static void mod_sysfs_teardown(struct module *mod) > * from modification and any data from execution. > * > * General layout of module is: > - * [text] [read-only-data] [writable data] > - * text_size -----^ ^ ^ > - * ro_size ------------------------| | > - * size -------------------------------------------| > + * [text] [read-only-data] [ro-after-init] [writable data] > + * text_size -----^ ^ ^ ^ > + * ro_size ------------------------| | | > + * ro_after_init_size -----------------------------| | > + * size -----------------------------------------------------------| > * > * These values are always page-aligned (as is base) > */ > @@ -1884,14 +1885,24 @@ static void frob_rodata(const struct module_layout *layout, > (layout->ro_size - layout->text_size) >> PAGE_SHIFT); > } > > +static void frob_ro_after_init(const struct module_layout *layout, > + int (*set_memory)(unsigned long start, int num_pages)) > +{ > + BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); > + BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); > + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); > + set_memory((unsigned long)layout->base + layout->ro_size, > + (layout->ro_after_init_size - layout->ro_size) >> PAGE_SHIFT); > +} > + > static void frob_writable_data(const struct module_layout *layout, > int (*set_memory)(unsigned long start, int num_pages)) > { > BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); > - BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); > + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); > BUG_ON((unsigned long)layout->size & (PAGE_SIZE-1)); > - set_memory((unsigned long)layout->base + layout->ro_size, > - (layout->size - layout->ro_size) >> PAGE_SHIFT); > + set_memory((unsigned long)layout->base + layout->ro_after_init_size, > + (layout->size - layout->ro_after_init_size) >> PAGE_SHIFT); > } > > /* livepatching wants to disable read-only so it can frob module. */ > @@ -1899,21 +1910,26 @@ void module_disable_ro(const struct module *mod) > { > frob_text(&mod->core_layout, set_memory_rw); > frob_rodata(&mod->core_layout, set_memory_rw); > + frob_ro_after_init(&mod->core_layout, set_memory_rw); > frob_text(&mod->init_layout, set_memory_rw); > frob_rodata(&mod->init_layout, set_memory_rw); > } > > -void module_enable_ro(const struct module *mod) > +void module_enable_ro(const struct module *mod, bool after_init) > { > frob_text(&mod->core_layout, set_memory_ro); > frob_rodata(&mod->core_layout, set_memory_ro); > frob_text(&mod->init_layout, set_memory_ro); > frob_rodata(&mod->init_layout, set_memory_ro); > + > + if (after_init) > + frob_ro_after_init(&mod->core_layout, set_memory_ro); > } > > static void module_enable_nx(const struct module *mod) > { > frob_rodata(&mod->core_layout, set_memory_nx); > + frob_ro_after_init(&mod->core_layout, set_memory_nx); > frob_writable_data(&mod->core_layout, set_memory_nx); > frob_rodata(&mod->init_layout, set_memory_nx); > frob_writable_data(&mod->init_layout, set_memory_nx); > @@ -1922,6 +1938,7 @@ static void module_enable_nx(const struct module *mod) > static void module_disable_nx(const struct module *mod) > { > frob_rodata(&mod->core_layout, set_memory_x); > + frob_ro_after_init(&mod->core_layout, set_memory_x); > frob_writable_data(&mod->core_layout, set_memory_x); > frob_rodata(&mod->init_layout, set_memory_x); > frob_writable_data(&mod->init_layout, set_memory_x); > @@ -1964,6 +1981,8 @@ static void disable_ro_nx(const struct module_layout *layout) > frob_text(layout, set_memory_rw); > frob_rodata(layout, set_memory_rw); > frob_rodata(layout, set_memory_x); > + frob_ro_after_init(layout, set_memory_rw); > + frob_ro_after_init(layout, set_memory_x); > frob_writable_data(layout, set_memory_x); > } > > @@ -2306,6 +2325,7 @@ static void layout_sections(struct module *mod, struct load_info *info) > * finder in the two loops below */ > { SHF_EXECINSTR | SHF_ALLOC, ARCH_SHF_SMALL }, > { SHF_ALLOC, SHF_WRITE | ARCH_SHF_SMALL }, > + { SHF_RO_AFTER_INIT | SHF_ALLOC, ARCH_SHF_SMALL }, > { SHF_WRITE | SHF_ALLOC, ARCH_SHF_SMALL }, > { ARCH_SHF_SMALL | SHF_ALLOC, 0 } > }; > @@ -2337,7 +2357,11 @@ static void layout_sections(struct module *mod, struct load_info *info) > mod->core_layout.size = debug_align(mod->core_layout.size); > mod->core_layout.ro_size = mod->core_layout.size; > break; > - case 3: /* whole core */ > + case 2: /* RO after init */ > + mod->core_layout.size = debug_align(mod->core_layout.size); > + mod->core_layout.ro_after_init_size = mod->core_layout.size; > + break; > + case 4: /* whole core */ > mod->core_layout.size = debug_align(mod->core_layout.size); > break; > } > @@ -2367,7 +2391,14 @@ static void layout_sections(struct module *mod, struct load_info *info) > mod->init_layout.size = debug_align(mod->init_layout.size); > mod->init_layout.ro_size = mod->init_layout.size; > break; > - case 3: /* whole init */ > + case 2: > + /* > + * RO after init doesn't apply to init_layout (only > + * core_layout), so it just takes the value of ro_size. > + */ > + mod->init_layout.ro_after_init_size = mod->init_layout.ro_size; > + break; > + case 4: /* whole init */ > mod->init_layout.size = debug_align(mod->init_layout.size); > break; > } > @@ -3173,6 +3204,7 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) > { > /* Module within temporary copy. */ > struct module *mod; > + unsigned int ndx; > int err; > > mod = setup_load_info(info, flags); > @@ -3192,6 +3224,15 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) > /* We will do a special allocation for per-cpu sections later. */ > info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC; > > + /* > + * Mark ro_after_init section with SHF_RO_AFTER_INIT so that > + * layout_sections() can put it in the right place. > + * Note: ro_after_init sections also have SHF_{WRITE,ALLOC} set. > + */ > + ndx = find_sec(info, ".data..ro_after_init"); > + if (ndx) > + info->sechdrs[ndx].sh_flags |= SHF_RO_AFTER_INIT; > + > /* Determine total sizes, and put offsets in sh_entsize. For now > this is done generically; there doesn't appear to be any > special cases for the architectures. */ > @@ -3358,12 +3399,14 @@ static noinline int do_init_module(struct module *mod) > /* Switch to core kallsyms now init is done: kallsyms may be walking! */ > rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms); > #endif > + module_enable_ro(mod, true); > mod_tree_remove_init(mod); > disable_ro_nx(&mod->init_layout); > module_arch_freeing_init(mod); > mod->init_layout.base = NULL; > mod->init_layout.size = 0; > mod->init_layout.ro_size = 0; > + mod->init_layout.ro_after_init_size = 0; > mod->init_layout.text_size = 0; > /* > * We want to free module_init, but be aware that kallsyms may be > @@ -3455,8 +3498,7 @@ static int complete_formation(struct module *mod, struct load_info *info) > /* This relies on module_mutex for list integrity. */ > module_bug_finalize(info->hdr, info->sechdrs, mod); > > - /* Set RO and NX regions */ > - module_enable_ro(mod); > + module_enable_ro(mod, false); > module_enable_nx(mod); > > /* Mark state as coming so strong_try_module_get() ignores us, > -- > 2.5.5 > -- Kees Cook Brillo & Chrome OS Security ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/1] Add ro_after_init support for modules @ 2016-06-14 0:13 Jessica Yu 2016-06-14 0:13 ` [PATCH 1/1] modules: add ro_after_init support Jessica Yu 0 siblings, 1 reply; 9+ messages in thread From: Jessica Yu @ 2016-06-14 0:13 UTC (permalink / raw) To: Rusty Russell, Kees Cook; +Cc: linux-api, linux-kernel, Jessica Yu Hi, This patch adds ro_after_init support for modules by adding an additional page-aligned section in the module layout. This new ro_after_init section sits between rodata and writable data. So, the new module layout looks like: [text] [rodata] [ro_after_init] [writable data] RO after init data remains RW during init and RO protection is enabled separately after module init runs. Did some light testing with lkdtm compiled as a module, verified that ro_after_init data is writable during init, and that it oopsed after attempted writes after init. Also tested livepatch (which uses module_{enable,disable}_ro for its own purposes) to make sure nothing broke. More testing is appreciated :-) Some remarks on the implementation: * A new SHF_RO_AFTER_INIT flag is introduced in elf.h to make identification of .data..ro_after_init sections and the work of layout_sections() easier. Its chosen value is within the SHF_MASKOS range. If people don't like adding a new SHF flag to elf.h, I could just make the flag internal to module.c. * frob_ro_after_init() could have been separated from module_enable_ro() (i.e., put it in its own function, something like module_enable_ro_after_init()), but given that livepatch also uses module_enable_ro(), I did not want to make livepatch worry about calling yet another function just to re-enable all RO protections for a module. * If a module doesn't have a ro_after_init section, then core_layout.ro_after_init_size just takes the value of core_layout.ro_size, and frob_ro_after_init() should do nothing. Based on linux-next. Previous discussion here: http://comments.gmane.org/gmane.linux.kernel/2234606 Jessica Yu (1): modules: add ro_after_init support include/linux/module.h | 2 ++ include/uapi/linux/elf.h | 1 + kernel/module.c | 73 +++++++++++++++++++++++++++++++++++++++++------- 3 files changed, 66 insertions(+), 10 deletions(-) -- 2.4.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] modules: add ro_after_init support 2016-06-14 0:13 [PATCH 0/1] Add ro_after_init support for modules Jessica Yu @ 2016-06-14 0:13 ` Jessica Yu 2016-06-14 21:33 ` Kees Cook 2016-06-29 1:08 ` [PATCH 1/1] " Rusty Russell 0 siblings, 2 replies; 9+ messages in thread From: Jessica Yu @ 2016-06-14 0:13 UTC (permalink / raw) To: Rusty Russell, Kees Cook; +Cc: linux-api, linux-kernel, Jessica Yu Add ro_after_init support for modules by adding a new page-aligned section in the module layout (after rodata) for ro_after_init data and enabling RO protection for that section after module init runs. Signed-off-by: Jessica Yu <jeyu@redhat.com> --- include/linux/module.h | 2 ++ include/uapi/linux/elf.h | 1 + kernel/module.c | 73 +++++++++++++++++++++++++++++++++++++++++------- 3 files changed, 66 insertions(+), 10 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index f777164..a698d23 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -311,6 +311,8 @@ struct module_layout { unsigned int text_size; /* Size of RO section of the module (text+rodata) */ unsigned int ro_size; + /* Size of RO after init section */ + unsigned int ro_after_init_size; #ifdef CONFIG_MODULES_TREE_LOOKUP struct mod_tree_node mtn; diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h index cb4a72f..70b172ba 100644 --- a/include/uapi/linux/elf.h +++ b/include/uapi/linux/elf.h @@ -286,6 +286,7 @@ typedef struct elf64_phdr { #define SHF_ALLOC 0x2 #define SHF_EXECINSTR 0x4 #define SHF_RELA_LIVEPATCH 0x00100000 +#define SHF_RO_AFTER_INIT 0x00200000 #define SHF_MASKPROC 0xf0000000 /* special section indexes */ diff --git a/kernel/module.c b/kernel/module.c index 7f21ab2..055bf6f 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1857,10 +1857,11 @@ static void mod_sysfs_teardown(struct module *mod) * from modification and any data from execution. * * General layout of module is: - * [text] [read-only-data] [writable data] - * text_size -----^ ^ ^ - * ro_size ------------------------| | - * size -------------------------------------------| + * [text] [read-only-data] [ro-after-init] [writable data] + * text_size -----^ ^ ^ ^ + * ro_size ------------------------| | | + * ro_after_init_size -----------------------------| | + * size -----------------------------------------------------------| * * These values are always page-aligned (as is base) */ @@ -1883,14 +1884,24 @@ static void frob_rodata(const struct module_layout *layout, (layout->ro_size - layout->text_size) >> PAGE_SHIFT); } +static void frob_ro_after_init(const struct module_layout *layout, + int (*set_memory)(unsigned long start, int num_pages)) +{ + BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); + BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); + set_memory((unsigned long)layout->base + layout->ro_size, + (layout->ro_after_init_size - layout->ro_size) >> PAGE_SHIFT); +} + static void frob_writable_data(const struct module_layout *layout, int (*set_memory)(unsigned long start, int num_pages)) { BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); - BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); BUG_ON((unsigned long)layout->size & (PAGE_SIZE-1)); - set_memory((unsigned long)layout->base + layout->ro_size, - (layout->size - layout->ro_size) >> PAGE_SHIFT); + set_memory((unsigned long)layout->base + layout->ro_after_init_size, + (layout->size - layout->ro_after_init_size) >> PAGE_SHIFT); } /* livepatching wants to disable read-only so it can frob module. */ @@ -1898,6 +1909,7 @@ void module_disable_ro(const struct module *mod) { frob_text(&mod->core_layout, set_memory_rw); frob_rodata(&mod->core_layout, set_memory_rw); + frob_ro_after_init(&mod->core_layout, set_memory_rw); frob_text(&mod->init_layout, set_memory_rw); frob_rodata(&mod->init_layout, set_memory_rw); } @@ -1908,11 +1920,16 @@ void module_enable_ro(const struct module *mod) frob_rodata(&mod->core_layout, set_memory_ro); frob_text(&mod->init_layout, set_memory_ro); frob_rodata(&mod->init_layout, set_memory_ro); + + /* after init */ + if (mod->state == MODULE_STATE_LIVE) + frob_ro_after_init(&mod->core_layout, set_memory_ro); } static void module_enable_nx(const struct module *mod) { frob_rodata(&mod->core_layout, set_memory_nx); + frob_ro_after_init(&mod->core_layout, set_memory_nx); frob_writable_data(&mod->core_layout, set_memory_nx); frob_rodata(&mod->init_layout, set_memory_nx); frob_writable_data(&mod->init_layout, set_memory_nx); @@ -1921,6 +1938,7 @@ static void module_enable_nx(const struct module *mod) static void module_disable_nx(const struct module *mod) { frob_rodata(&mod->core_layout, set_memory_x); + frob_ro_after_init(&mod->core_layout, set_memory_x); frob_writable_data(&mod->core_layout, set_memory_x); frob_rodata(&mod->init_layout, set_memory_x); frob_writable_data(&mod->init_layout, set_memory_x); @@ -1963,6 +1981,8 @@ static void disable_ro_nx(const struct module_layout *layout) frob_text(layout, set_memory_rw); frob_rodata(layout, set_memory_rw); frob_rodata(layout, set_memory_x); + frob_ro_after_init(layout, set_memory_rw); + frob_ro_after_init(layout, set_memory_x); frob_writable_data(layout, set_memory_x); } @@ -2305,6 +2325,7 @@ static void layout_sections(struct module *mod, struct load_info *info) * finder in the two loops below */ { SHF_EXECINSTR | SHF_ALLOC, ARCH_SHF_SMALL }, { SHF_ALLOC, SHF_WRITE | ARCH_SHF_SMALL }, + { SHF_RO_AFTER_INIT | SHF_ALLOC, ARCH_SHF_SMALL }, { SHF_WRITE | SHF_ALLOC, ARCH_SHF_SMALL }, { ARCH_SHF_SMALL | SHF_ALLOC, 0 } }; @@ -2336,7 +2357,11 @@ static void layout_sections(struct module *mod, struct load_info *info) mod->core_layout.size = debug_align(mod->core_layout.size); mod->core_layout.ro_size = mod->core_layout.size; break; - case 3: /* whole core */ + case 2: /* RO after init */ + mod->core_layout.size = debug_align(mod->core_layout.size); + mod->core_layout.ro_after_init_size = mod->core_layout.size; + break; + case 4: /* whole core */ mod->core_layout.size = debug_align(mod->core_layout.size); break; } @@ -2366,7 +2391,14 @@ static void layout_sections(struct module *mod, struct load_info *info) mod->init_layout.size = debug_align(mod->init_layout.size); mod->init_layout.ro_size = mod->init_layout.size; break; - case 3: /* whole init */ + case 2: + /* + * RO after init doesn't apply to init_layout (only + * core_layout), so it just takes the value of ro_size. + */ + mod->init_layout.ro_after_init_size = mod->init_layout.ro_size; + break; + case 4: /* whole init */ mod->init_layout.size = debug_align(mod->init_layout.size); break; } @@ -3172,6 +3204,7 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) { /* Module within temporary copy. */ struct module *mod; + unsigned int ndx; int err; mod = setup_load_info(info, flags); @@ -3191,6 +3224,15 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) /* We will do a special allocation for per-cpu sections later. */ info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC; + /* + * Mark ro_after_init section with SHF_RO_AFTER_INIT so that + * layout_sections() can put it in the right place. + * Note: ro_after_init sections also have SHF_{WRITE,ALLOC} set. + */ + ndx = find_sec(info, ".data..ro_after_init"); + if (ndx) + info->sechdrs[ndx].sh_flags |= SHF_RO_AFTER_INIT; + /* Determine total sizes, and put offsets in sh_entsize. For now this is done generically; there doesn't appear to be any special cases for the architectures. */ @@ -3357,12 +3399,19 @@ static noinline int do_init_module(struct module *mod) /* Switch to core kallsyms now init is done: kallsyms may be walking! */ rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms); #endif + /* + * RO and NX regions should already be protected at this point, + * so the below module_enable_ro() call enables additional RO + * protection for the ro_after_init section only. + */ + module_enable_ro(mod); mod_tree_remove_init(mod); disable_ro_nx(&mod->init_layout); module_arch_freeing_init(mod); mod->init_layout.base = NULL; mod->init_layout.size = 0; mod->init_layout.ro_size = 0; + mod->init_layout.ro_after_init_size = 0; mod->init_layout.text_size = 0; /* * We want to free module_init, but be aware that kallsyms may be @@ -3454,7 +3503,11 @@ static int complete_formation(struct module *mod, struct load_info *info) /* This relies on module_mutex for list integrity. */ module_bug_finalize(info->hdr, info->sechdrs, mod); - /* Set RO and NX regions */ + /* + * Set RO and NX regions. Since module is not LIVE yet, + * the ro_after_init section will remain RW until after + * do_one_initcall(). + */ module_enable_ro(mod); module_enable_nx(mod); -- 2.4.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] modules: add ro_after_init support 2016-06-14 0:13 ` [PATCH 1/1] modules: add ro_after_init support Jessica Yu @ 2016-06-14 21:33 ` Kees Cook 2016-06-14 23:53 ` Jessica Yu 2016-06-29 1:08 ` [PATCH 1/1] " Rusty Russell 1 sibling, 1 reply; 9+ messages in thread From: Kees Cook @ 2016-06-14 21:33 UTC (permalink / raw) To: Jessica Yu; +Cc: Rusty Russell, Linux API, LKML On Mon, Jun 13, 2016 at 5:13 PM, Jessica Yu <jeyu@redhat.com> wrote: > Add ro_after_init support for modules by adding a new page-aligned section > in the module layout (after rodata) for ro_after_init data and enabling RO > protection for that section after module init runs. > > Signed-off-by: Jessica Yu <jeyu@redhat.com> This looks awesome! Thanks for working on it. > --- > include/linux/module.h | 2 ++ > include/uapi/linux/elf.h | 1 + > kernel/module.c | 73 +++++++++++++++++++++++++++++++++++++++++------- > 3 files changed, 66 insertions(+), 10 deletions(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index f777164..a698d23 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -311,6 +311,8 @@ struct module_layout { > unsigned int text_size; > /* Size of RO section of the module (text+rodata) */ > unsigned int ro_size; > + /* Size of RO after init section */ > + unsigned int ro_after_init_size; > > #ifdef CONFIG_MODULES_TREE_LOOKUP > struct mod_tree_node mtn; > diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h > index cb4a72f..70b172ba 100644 > --- a/include/uapi/linux/elf.h > +++ b/include/uapi/linux/elf.h > @@ -286,6 +286,7 @@ typedef struct elf64_phdr { > #define SHF_ALLOC 0x2 > #define SHF_EXECINSTR 0x4 > #define SHF_RELA_LIVEPATCH 0x00100000 > +#define SHF_RO_AFTER_INIT 0x00200000 This gives us some flexibility in the ELF flags, but it seems like overkill since nothing will actually be setting it externally. I defer to you and Rusty on this one. :) > #define SHF_MASKPROC 0xf0000000 > > /* special section indexes */ > diff --git a/kernel/module.c b/kernel/module.c > index 7f21ab2..055bf6f 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -1857,10 +1857,11 @@ static void mod_sysfs_teardown(struct module *mod) > * from modification and any data from execution. > * > * General layout of module is: > - * [text] [read-only-data] [writable data] > - * text_size -----^ ^ ^ > - * ro_size ------------------------| | > - * size -------------------------------------------| > + * [text] [read-only-data] [ro-after-init] [writable data] > + * text_size -----^ ^ ^ ^ > + * ro_size ------------------------| | | > + * ro_after_init_size -----------------------------| | > + * size -----------------------------------------------------------| > * > * These values are always page-aligned (as is base) > */ > @@ -1883,14 +1884,24 @@ static void frob_rodata(const struct module_layout *layout, > (layout->ro_size - layout->text_size) >> PAGE_SHIFT); > } > > +static void frob_ro_after_init(const struct module_layout *layout, > + int (*set_memory)(unsigned long start, int num_pages)) > +{ > + BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); > + BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); > + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); > + set_memory((unsigned long)layout->base + layout->ro_size, > + (layout->ro_after_init_size - layout->ro_size) >> PAGE_SHIFT); > +} > + > static void frob_writable_data(const struct module_layout *layout, > int (*set_memory)(unsigned long start, int num_pages)) > { > BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); > - BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); > + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); > BUG_ON((unsigned long)layout->size & (PAGE_SIZE-1)); > - set_memory((unsigned long)layout->base + layout->ro_size, > - (layout->size - layout->ro_size) >> PAGE_SHIFT); > + set_memory((unsigned long)layout->base + layout->ro_after_init_size, > + (layout->size - layout->ro_after_init_size) >> PAGE_SHIFT); > } > > /* livepatching wants to disable read-only so it can frob module. */ > @@ -1898,6 +1909,7 @@ void module_disable_ro(const struct module *mod) > { > frob_text(&mod->core_layout, set_memory_rw); > frob_rodata(&mod->core_layout, set_memory_rw); > + frob_ro_after_init(&mod->core_layout, set_memory_rw); > frob_text(&mod->init_layout, set_memory_rw); > frob_rodata(&mod->init_layout, set_memory_rw); > } > @@ -1908,11 +1920,16 @@ void module_enable_ro(const struct module *mod) > frob_rodata(&mod->core_layout, set_memory_ro); > frob_text(&mod->init_layout, set_memory_ro); > frob_rodata(&mod->init_layout, set_memory_ro); > + > + /* after init */ > + if (mod->state == MODULE_STATE_LIVE) > + frob_ro_after_init(&mod->core_layout, set_memory_ro); > } > > static void module_enable_nx(const struct module *mod) > { > frob_rodata(&mod->core_layout, set_memory_nx); > + frob_ro_after_init(&mod->core_layout, set_memory_nx); > frob_writable_data(&mod->core_layout, set_memory_nx); > frob_rodata(&mod->init_layout, set_memory_nx); > frob_writable_data(&mod->init_layout, set_memory_nx); > @@ -1921,6 +1938,7 @@ static void module_enable_nx(const struct module *mod) > static void module_disable_nx(const struct module *mod) > { > frob_rodata(&mod->core_layout, set_memory_x); > + frob_ro_after_init(&mod->core_layout, set_memory_x); > frob_writable_data(&mod->core_layout, set_memory_x); > frob_rodata(&mod->init_layout, set_memory_x); > frob_writable_data(&mod->init_layout, set_memory_x); > @@ -1963,6 +1981,8 @@ static void disable_ro_nx(const struct module_layout *layout) > frob_text(layout, set_memory_rw); > frob_rodata(layout, set_memory_rw); > frob_rodata(layout, set_memory_x); > + frob_ro_after_init(layout, set_memory_rw); > + frob_ro_after_init(layout, set_memory_x); > frob_writable_data(layout, set_memory_x); > } > > @@ -2305,6 +2325,7 @@ static void layout_sections(struct module *mod, struct load_info *info) > * finder in the two loops below */ > { SHF_EXECINSTR | SHF_ALLOC, ARCH_SHF_SMALL }, > { SHF_ALLOC, SHF_WRITE | ARCH_SHF_SMALL }, > + { SHF_RO_AFTER_INIT | SHF_ALLOC, ARCH_SHF_SMALL }, > { SHF_WRITE | SHF_ALLOC, ARCH_SHF_SMALL }, > { ARCH_SHF_SMALL | SHF_ALLOC, 0 } > }; > @@ -2336,7 +2357,11 @@ static void layout_sections(struct module *mod, struct load_info *info) > mod->core_layout.size = debug_align(mod->core_layout.size); > mod->core_layout.ro_size = mod->core_layout.size; > break; > - case 3: /* whole core */ > + case 2: /* RO after init */ > + mod->core_layout.size = debug_align(mod->core_layout.size); > + mod->core_layout.ro_after_init_size = mod->core_layout.size; > + break; > + case 4: /* whole core */ > mod->core_layout.size = debug_align(mod->core_layout.size); > break; > } > @@ -2366,7 +2391,14 @@ static void layout_sections(struct module *mod, struct load_info *info) > mod->init_layout.size = debug_align(mod->init_layout.size); > mod->init_layout.ro_size = mod->init_layout.size; > break; > - case 3: /* whole init */ > + case 2: > + /* > + * RO after init doesn't apply to init_layout (only > + * core_layout), so it just takes the value of ro_size. > + */ > + mod->init_layout.ro_after_init_size = mod->init_layout.ro_size; > + break; > + case 4: /* whole init */ > mod->init_layout.size = debug_align(mod->init_layout.size); > break; > } > @@ -3172,6 +3204,7 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) > { > /* Module within temporary copy. */ > struct module *mod; > + unsigned int ndx; > int err; > > mod = setup_load_info(info, flags); > @@ -3191,6 +3224,15 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) > /* We will do a special allocation for per-cpu sections later. */ > info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC; > > + /* > + * Mark ro_after_init section with SHF_RO_AFTER_INIT so that > + * layout_sections() can put it in the right place. > + * Note: ro_after_init sections also have SHF_{WRITE,ALLOC} set. > + */ > + ndx = find_sec(info, ".data..ro_after_init"); > + if (ndx) > + info->sechdrs[ndx].sh_flags |= SHF_RO_AFTER_INIT; > + > /* Determine total sizes, and put offsets in sh_entsize. For now > this is done generically; there doesn't appear to be any > special cases for the architectures. */ > @@ -3357,12 +3399,19 @@ static noinline int do_init_module(struct module *mod) > /* Switch to core kallsyms now init is done: kallsyms may be walking! */ > rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms); > #endif > + /* > + * RO and NX regions should already be protected at this point, > + * so the below module_enable_ro() call enables additional RO > + * protection for the ro_after_init section only. > + */ > + module_enable_ro(mod); My only thought here is that this seems like needless setting of all the already-set regions. I'm fine with this personally, but I wonder if maybe it would be better to just call the frob instead? frob_ro_after_init(&mod->core_layout, set_memory_ro); > mod_tree_remove_init(mod); > disable_ro_nx(&mod->init_layout); > module_arch_freeing_init(mod); > mod->init_layout.base = NULL; > mod->init_layout.size = 0; > mod->init_layout.ro_size = 0; > + mod->init_layout.ro_after_init_size = 0; > mod->init_layout.text_size = 0; > /* > * We want to free module_init, but be aware that kallsyms may be > @@ -3454,7 +3503,11 @@ static int complete_formation(struct module *mod, struct load_info *info) > /* This relies on module_mutex for list integrity. */ > module_bug_finalize(info->hdr, info->sechdrs, mod); > > - /* Set RO and NX regions */ > + /* > + * Set RO and NX regions. Since module is not LIVE yet, > + * the ro_after_init section will remain RW until after > + * do_one_initcall(). > + */ > module_enable_ro(mod); > module_enable_nx(mod); > > -- > 2.4.3 > Regardless, either way: Reviewed-by: Kees Cook <keescook@chromium.org> -Kees -- Kees Cook Chrome OS & Brillo Security ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: modules: add ro_after_init support 2016-06-14 21:33 ` Kees Cook @ 2016-06-14 23:53 ` Jessica Yu 0 siblings, 0 replies; 9+ messages in thread From: Jessica Yu @ 2016-06-14 23:53 UTC (permalink / raw) To: Kees Cook; +Cc: Rusty Russell, Linux API, LKML +++ Kees Cook [14/06/16 14:33 -0700]: >On Mon, Jun 13, 2016 at 5:13 PM, Jessica Yu <jeyu@redhat.com> wrote: >> Add ro_after_init support for modules by adding a new page-aligned section >> in the module layout (after rodata) for ro_after_init data and enabling RO >> protection for that section after module init runs. >> >> Signed-off-by: Jessica Yu <jeyu@redhat.com> > >This looks awesome! Thanks for working on it. > >> --- >> include/linux/module.h | 2 ++ >> include/uapi/linux/elf.h | 1 + >> kernel/module.c | 73 +++++++++++++++++++++++++++++++++++++++++------- >> 3 files changed, 66 insertions(+), 10 deletions(-) >> >> diff --git a/include/linux/module.h b/include/linux/module.h >> index f777164..a698d23 100644 >> --- a/include/linux/module.h >> +++ b/include/linux/module.h >> @@ -311,6 +311,8 @@ struct module_layout { >> unsigned int text_size; >> /* Size of RO section of the module (text+rodata) */ >> unsigned int ro_size; >> + /* Size of RO after init section */ >> + unsigned int ro_after_init_size; >> >> #ifdef CONFIG_MODULES_TREE_LOOKUP >> struct mod_tree_node mtn; >> diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h >> index cb4a72f..70b172ba 100644 >> --- a/include/uapi/linux/elf.h >> +++ b/include/uapi/linux/elf.h >> @@ -286,6 +286,7 @@ typedef struct elf64_phdr { >> #define SHF_ALLOC 0x2 >> #define SHF_EXECINSTR 0x4 >> #define SHF_RELA_LIVEPATCH 0x00100000 >> +#define SHF_RO_AFTER_INIT 0x00200000 > >This gives us some flexibility in the ELF flags, but it seems like >overkill since nothing will actually be setting it externally. I defer >to you and Rusty on this one. :) Hm yeah, the flag would only be used in the module code, so maybe it should be internal to module.c.. >> #define SHF_MASKPROC 0xf0000000 >> >> /* special section indexes */ >> diff --git a/kernel/module.c b/kernel/module.c >> index 7f21ab2..055bf6f 100644 >> --- a/kernel/module.c >> +++ b/kernel/module.c >> @@ -1857,10 +1857,11 @@ static void mod_sysfs_teardown(struct module *mod) >> * from modification and any data from execution. >> * >> * General layout of module is: >> - * [text] [read-only-data] [writable data] >> - * text_size -----^ ^ ^ >> - * ro_size ------------------------| | >> - * size -------------------------------------------| >> + * [text] [read-only-data] [ro-after-init] [writable data] >> + * text_size -----^ ^ ^ ^ >> + * ro_size ------------------------| | | >> + * ro_after_init_size -----------------------------| | >> + * size -----------------------------------------------------------| >> * >> * These values are always page-aligned (as is base) >> */ >> @@ -1883,14 +1884,24 @@ static void frob_rodata(const struct module_layout *layout, >> (layout->ro_size - layout->text_size) >> PAGE_SHIFT); >> } >> >> +static void frob_ro_after_init(const struct module_layout *layout, >> + int (*set_memory)(unsigned long start, int num_pages)) >> +{ >> + BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); >> + BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); >> + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); >> + set_memory((unsigned long)layout->base + layout->ro_size, >> + (layout->ro_after_init_size - layout->ro_size) >> PAGE_SHIFT); >> +} >> + >> static void frob_writable_data(const struct module_layout *layout, >> int (*set_memory)(unsigned long start, int num_pages)) >> { >> BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); >> - BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); >> + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); >> BUG_ON((unsigned long)layout->size & (PAGE_SIZE-1)); >> - set_memory((unsigned long)layout->base + layout->ro_size, >> - (layout->size - layout->ro_size) >> PAGE_SHIFT); >> + set_memory((unsigned long)layout->base + layout->ro_after_init_size, >> + (layout->size - layout->ro_after_init_size) >> PAGE_SHIFT); >> } >> >> /* livepatching wants to disable read-only so it can frob module. */ >> @@ -1898,6 +1909,7 @@ void module_disable_ro(const struct module *mod) >> { >> frob_text(&mod->core_layout, set_memory_rw); >> frob_rodata(&mod->core_layout, set_memory_rw); >> + frob_ro_after_init(&mod->core_layout, set_memory_rw); >> frob_text(&mod->init_layout, set_memory_rw); >> frob_rodata(&mod->init_layout, set_memory_rw); >> } >> @@ -1908,11 +1920,16 @@ void module_enable_ro(const struct module *mod) >> frob_rodata(&mod->core_layout, set_memory_ro); >> frob_text(&mod->init_layout, set_memory_ro); >> frob_rodata(&mod->init_layout, set_memory_ro); >> + >> + /* after init */ >> + if (mod->state == MODULE_STATE_LIVE) >> + frob_ro_after_init(&mod->core_layout, set_memory_ro); >> } >> >> static void module_enable_nx(const struct module *mod) >> { >> frob_rodata(&mod->core_layout, set_memory_nx); >> + frob_ro_after_init(&mod->core_layout, set_memory_nx); >> frob_writable_data(&mod->core_layout, set_memory_nx); >> frob_rodata(&mod->init_layout, set_memory_nx); >> frob_writable_data(&mod->init_layout, set_memory_nx); >> @@ -1921,6 +1938,7 @@ static void module_enable_nx(const struct module *mod) >> static void module_disable_nx(const struct module *mod) >> { >> frob_rodata(&mod->core_layout, set_memory_x); >> + frob_ro_after_init(&mod->core_layout, set_memory_x); >> frob_writable_data(&mod->core_layout, set_memory_x); >> frob_rodata(&mod->init_layout, set_memory_x); >> frob_writable_data(&mod->init_layout, set_memory_x); >> @@ -1963,6 +1981,8 @@ static void disable_ro_nx(const struct module_layout *layout) >> frob_text(layout, set_memory_rw); >> frob_rodata(layout, set_memory_rw); >> frob_rodata(layout, set_memory_x); >> + frob_ro_after_init(layout, set_memory_rw); >> + frob_ro_after_init(layout, set_memory_x); >> frob_writable_data(layout, set_memory_x); >> } >> >> @@ -2305,6 +2325,7 @@ static void layout_sections(struct module *mod, struct load_info *info) >> * finder in the two loops below */ >> { SHF_EXECINSTR | SHF_ALLOC, ARCH_SHF_SMALL }, >> { SHF_ALLOC, SHF_WRITE | ARCH_SHF_SMALL }, >> + { SHF_RO_AFTER_INIT | SHF_ALLOC, ARCH_SHF_SMALL }, >> { SHF_WRITE | SHF_ALLOC, ARCH_SHF_SMALL }, >> { ARCH_SHF_SMALL | SHF_ALLOC, 0 } >> }; >> @@ -2336,7 +2357,11 @@ static void layout_sections(struct module *mod, struct load_info *info) >> mod->core_layout.size = debug_align(mod->core_layout.size); >> mod->core_layout.ro_size = mod->core_layout.size; >> break; >> - case 3: /* whole core */ >> + case 2: /* RO after init */ >> + mod->core_layout.size = debug_align(mod->core_layout.size); >> + mod->core_layout.ro_after_init_size = mod->core_layout.size; >> + break; >> + case 4: /* whole core */ >> mod->core_layout.size = debug_align(mod->core_layout.size); >> break; >> } >> @@ -2366,7 +2391,14 @@ static void layout_sections(struct module *mod, struct load_info *info) >> mod->init_layout.size = debug_align(mod->init_layout.size); >> mod->init_layout.ro_size = mod->init_layout.size; >> break; >> - case 3: /* whole init */ >> + case 2: >> + /* >> + * RO after init doesn't apply to init_layout (only >> + * core_layout), so it just takes the value of ro_size. >> + */ >> + mod->init_layout.ro_after_init_size = mod->init_layout.ro_size; >> + break; >> + case 4: /* whole init */ >> mod->init_layout.size = debug_align(mod->init_layout.size); >> break; >> } >> @@ -3172,6 +3204,7 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) >> { >> /* Module within temporary copy. */ >> struct module *mod; >> + unsigned int ndx; >> int err; >> >> mod = setup_load_info(info, flags); >> @@ -3191,6 +3224,15 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) >> /* We will do a special allocation for per-cpu sections later. */ >> info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC; >> >> + /* >> + * Mark ro_after_init section with SHF_RO_AFTER_INIT so that >> + * layout_sections() can put it in the right place. >> + * Note: ro_after_init sections also have SHF_{WRITE,ALLOC} set. >> + */ >> + ndx = find_sec(info, ".data..ro_after_init"); >> + if (ndx) >> + info->sechdrs[ndx].sh_flags |= SHF_RO_AFTER_INIT; >> + >> /* Determine total sizes, and put offsets in sh_entsize. For now >> this is done generically; there doesn't appear to be any >> special cases for the architectures. */ >> @@ -3357,12 +3399,19 @@ static noinline int do_init_module(struct module *mod) >> /* Switch to core kallsyms now init is done: kallsyms may be walking! */ >> rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms); >> #endif >> + /* >> + * RO and NX regions should already be protected at this point, >> + * so the below module_enable_ro() call enables additional RO >> + * protection for the ro_after_init section only. >> + */ >> + module_enable_ro(mod); > >My only thought here is that this seems like needless setting of all >the already-set regions. I'm fine with this personally, but I wonder >if maybe it would be better to just call the frob instead? > > frob_ro_after_init(&mod->core_layout, set_memory_ro); Yeah, that works too. :-) I figured since module_enable_ro() needs to call frob_ro_after_init() anyway (because livepatch is also a user of module_{enable,disable}_ro), it wouldn't hurt to call module_enable_ro() again here after mod state is LIVE. But maybe a direct call to frob_ro_after_init() is less confusing. >> mod_tree_remove_init(mod); >> disable_ro_nx(&mod->init_layout); >> module_arch_freeing_init(mod); >> mod->init_layout.base = NULL; >> mod->init_layout.size = 0; >> mod->init_layout.ro_size = 0; >> + mod->init_layout.ro_after_init_size = 0; >> mod->init_layout.text_size = 0; >> /* >> * We want to free module_init, but be aware that kallsyms may be >> @@ -3454,7 +3503,11 @@ static int complete_formation(struct module *mod, struct load_info *info) >> /* This relies on module_mutex for list integrity. */ >> module_bug_finalize(info->hdr, info->sechdrs, mod); >> >> - /* Set RO and NX regions */ >> + /* >> + * Set RO and NX regions. Since module is not LIVE yet, >> + * the ro_after_init section will remain RW until after >> + * do_one_initcall(). >> + */ >> module_enable_ro(mod); >> module_enable_nx(mod); >> >> -- >> 2.4.3 >> > >Regardless, either way: > >Reviewed-by: Kees Cook <keescook@chromium.org> Thanks for the review! Jessica ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] modules: add ro_after_init support 2016-06-14 0:13 ` [PATCH 1/1] modules: add ro_after_init support Jessica Yu 2016-06-14 21:33 ` Kees Cook @ 2016-06-29 1:08 ` Rusty Russell 2016-06-29 21:27 ` Jessica Yu 1 sibling, 1 reply; 9+ messages in thread From: Rusty Russell @ 2016-06-29 1:08 UTC (permalink / raw) To: Jessica Yu, Kees Cook; +Cc: linux-api, linux-kernel, Jessica Yu Jessica Yu <jeyu@redhat.com> writes: > Add ro_after_init support for modules by adding a new page-aligned section > in the module layout (after rodata) for ro_after_init data and enabling RO > protection for that section after module init runs. > > Signed-off-by: Jessica Yu <jeyu@redhat.com> I would prefer a "bool after_init" flag to module_enable_ro(). It's more explicit. Exposing the flags via uapi looks like a wart, but it's kind of a feature, since we don't *unset* it in any section; userspace may want to know about it. If you could re-spin, that would be great. Thanks! Rusty. > --- > include/linux/module.h | 2 ++ > include/uapi/linux/elf.h | 1 + > kernel/module.c | 73 +++++++++++++++++++++++++++++++++++++++++------- > 3 files changed, 66 insertions(+), 10 deletions(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index f777164..a698d23 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -311,6 +311,8 @@ struct module_layout { > unsigned int text_size; > /* Size of RO section of the module (text+rodata) */ > unsigned int ro_size; > + /* Size of RO after init section */ > + unsigned int ro_after_init_size; > > #ifdef CONFIG_MODULES_TREE_LOOKUP > struct mod_tree_node mtn; > diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h > index cb4a72f..70b172ba 100644 > --- a/include/uapi/linux/elf.h > +++ b/include/uapi/linux/elf.h > @@ -286,6 +286,7 @@ typedef struct elf64_phdr { > #define SHF_ALLOC 0x2 > #define SHF_EXECINSTR 0x4 > #define SHF_RELA_LIVEPATCH 0x00100000 > +#define SHF_RO_AFTER_INIT 0x00200000 > #define SHF_MASKPROC 0xf0000000 > > /* special section indexes */ > diff --git a/kernel/module.c b/kernel/module.c > index 7f21ab2..055bf6f 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -1857,10 +1857,11 @@ static void mod_sysfs_teardown(struct module *mod) > * from modification and any data from execution. > * > * General layout of module is: > - * [text] [read-only-data] [writable data] > - * text_size -----^ ^ ^ > - * ro_size ------------------------| | > - * size -------------------------------------------| > + * [text] [read-only-data] [ro-after-init] [writable data] > + * text_size -----^ ^ ^ ^ > + * ro_size ------------------------| | | > + * ro_after_init_size -----------------------------| | > + * size -----------------------------------------------------------| > * > * These values are always page-aligned (as is base) > */ > @@ -1883,14 +1884,24 @@ static void frob_rodata(const struct module_layout *layout, > (layout->ro_size - layout->text_size) >> PAGE_SHIFT); > } > > +static void frob_ro_after_init(const struct module_layout *layout, > + int (*set_memory)(unsigned long start, int num_pages)) > +{ > + BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); > + BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); > + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); > + set_memory((unsigned long)layout->base + layout->ro_size, > + (layout->ro_after_init_size - layout->ro_size) >> PAGE_SHIFT); > +} > + > static void frob_writable_data(const struct module_layout *layout, > int (*set_memory)(unsigned long start, int num_pages)) > { > BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); > - BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); > + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); > BUG_ON((unsigned long)layout->size & (PAGE_SIZE-1)); > - set_memory((unsigned long)layout->base + layout->ro_size, > - (layout->size - layout->ro_size) >> PAGE_SHIFT); > + set_memory((unsigned long)layout->base + layout->ro_after_init_size, > + (layout->size - layout->ro_after_init_size) >> PAGE_SHIFT); > } > > /* livepatching wants to disable read-only so it can frob module. */ > @@ -1898,6 +1909,7 @@ void module_disable_ro(const struct module *mod) > { > frob_text(&mod->core_layout, set_memory_rw); > frob_rodata(&mod->core_layout, set_memory_rw); > + frob_ro_after_init(&mod->core_layout, set_memory_rw); > frob_text(&mod->init_layout, set_memory_rw); > frob_rodata(&mod->init_layout, set_memory_rw); > } > @@ -1908,11 +1920,16 @@ void module_enable_ro(const struct module *mod) > frob_rodata(&mod->core_layout, set_memory_ro); > frob_text(&mod->init_layout, set_memory_ro); > frob_rodata(&mod->init_layout, set_memory_ro); > + > + /* after init */ > + if (mod->state == MODULE_STATE_LIVE) > + frob_ro_after_init(&mod->core_layout, set_memory_ro); > } > > static void module_enable_nx(const struct module *mod) > { > frob_rodata(&mod->core_layout, set_memory_nx); > + frob_ro_after_init(&mod->core_layout, set_memory_nx); > frob_writable_data(&mod->core_layout, set_memory_nx); > frob_rodata(&mod->init_layout, set_memory_nx); > frob_writable_data(&mod->init_layout, set_memory_nx); > @@ -1921,6 +1938,7 @@ static void module_enable_nx(const struct module *mod) > static void module_disable_nx(const struct module *mod) > { > frob_rodata(&mod->core_layout, set_memory_x); > + frob_ro_after_init(&mod->core_layout, set_memory_x); > frob_writable_data(&mod->core_layout, set_memory_x); > frob_rodata(&mod->init_layout, set_memory_x); > frob_writable_data(&mod->init_layout, set_memory_x); > @@ -1963,6 +1981,8 @@ static void disable_ro_nx(const struct module_layout *layout) > frob_text(layout, set_memory_rw); > frob_rodata(layout, set_memory_rw); > frob_rodata(layout, set_memory_x); > + frob_ro_after_init(layout, set_memory_rw); > + frob_ro_after_init(layout, set_memory_x); > frob_writable_data(layout, set_memory_x); > } > > @@ -2305,6 +2325,7 @@ static void layout_sections(struct module *mod, struct load_info *info) > * finder in the two loops below */ > { SHF_EXECINSTR | SHF_ALLOC, ARCH_SHF_SMALL }, > { SHF_ALLOC, SHF_WRITE | ARCH_SHF_SMALL }, > + { SHF_RO_AFTER_INIT | SHF_ALLOC, ARCH_SHF_SMALL }, > { SHF_WRITE | SHF_ALLOC, ARCH_SHF_SMALL }, > { ARCH_SHF_SMALL | SHF_ALLOC, 0 } > }; > @@ -2336,7 +2357,11 @@ static void layout_sections(struct module *mod, struct load_info *info) > mod->core_layout.size = debug_align(mod->core_layout.size); > mod->core_layout.ro_size = mod->core_layout.size; > break; > - case 3: /* whole core */ > + case 2: /* RO after init */ > + mod->core_layout.size = debug_align(mod->core_layout.size); > + mod->core_layout.ro_after_init_size = mod->core_layout.size; > + break; > + case 4: /* whole core */ > mod->core_layout.size = debug_align(mod->core_layout.size); > break; > } > @@ -2366,7 +2391,14 @@ static void layout_sections(struct module *mod, struct load_info *info) > mod->init_layout.size = debug_align(mod->init_layout.size); > mod->init_layout.ro_size = mod->init_layout.size; > break; > - case 3: /* whole init */ > + case 2: > + /* > + * RO after init doesn't apply to init_layout (only > + * core_layout), so it just takes the value of ro_size. > + */ > + mod->init_layout.ro_after_init_size = mod->init_layout.ro_size; > + break; > + case 4: /* whole init */ > mod->init_layout.size = debug_align(mod->init_layout.size); > break; > } > @@ -3172,6 +3204,7 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) > { > /* Module within temporary copy. */ > struct module *mod; > + unsigned int ndx; > int err; > > mod = setup_load_info(info, flags); > @@ -3191,6 +3224,15 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) > /* We will do a special allocation for per-cpu sections later. */ > info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC; > > + /* > + * Mark ro_after_init section with SHF_RO_AFTER_INIT so that > + * layout_sections() can put it in the right place. > + * Note: ro_after_init sections also have SHF_{WRITE,ALLOC} set. > + */ > + ndx = find_sec(info, ".data..ro_after_init"); > + if (ndx) > + info->sechdrs[ndx].sh_flags |= SHF_RO_AFTER_INIT; > + > /* Determine total sizes, and put offsets in sh_entsize. For now > this is done generically; there doesn't appear to be any > special cases for the architectures. */ > @@ -3357,12 +3399,19 @@ static noinline int do_init_module(struct module *mod) > /* Switch to core kallsyms now init is done: kallsyms may be walking! */ > rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms); > #endif > + /* > + * RO and NX regions should already be protected at this point, > + * so the below module_enable_ro() call enables additional RO > + * protection for the ro_after_init section only. > + */ > + module_enable_ro(mod); > mod_tree_remove_init(mod); > disable_ro_nx(&mod->init_layout); > module_arch_freeing_init(mod); > mod->init_layout.base = NULL; > mod->init_layout.size = 0; > mod->init_layout.ro_size = 0; > + mod->init_layout.ro_after_init_size = 0; > mod->init_layout.text_size = 0; > /* > * We want to free module_init, but be aware that kallsyms may be > @@ -3454,7 +3503,11 @@ static int complete_formation(struct module *mod, struct load_info *info) > /* This relies on module_mutex for list integrity. */ > module_bug_finalize(info->hdr, info->sechdrs, mod); > > - /* Set RO and NX regions */ > + /* > + * Set RO and NX regions. Since module is not LIVE yet, > + * the ro_after_init section will remain RW until after > + * do_one_initcall(). > + */ > module_enable_ro(mod); > module_enable_nx(mod); > > -- > 2.4.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: modules: add ro_after_init support 2016-06-29 1:08 ` [PATCH 1/1] " Rusty Russell @ 2016-06-29 21:27 ` Jessica Yu 2016-06-30 4:56 ` Rusty Russell 0 siblings, 1 reply; 9+ messages in thread From: Jessica Yu @ 2016-06-29 21:27 UTC (permalink / raw) To: Rusty Russell; +Cc: Kees Cook, linux-api, linux-kernel +++ Rusty Russell [29/06/16 10:38 +0930]: >Jessica Yu <jeyu@redhat.com> writes: >> Add ro_after_init support for modules by adding a new page-aligned section >> in the module layout (after rodata) for ro_after_init data and enabling RO >> protection for that section after module init runs. >> >> Signed-off-by: Jessica Yu <jeyu@redhat.com> > >I would prefer a "bool after_init" flag to module_enable_ro(). It's >more explicit. Sure thing, I was just initially worried about the module_{enable,disable}_ro() asymmetry. :) >Exposing the flags via uapi looks like a wart, but it's kind of a >feature, since we don't *unset* it in any section; userspace may want to >know about it. Hm, I'm still unsure about this. I'm starting to think it might be a bit overkill to expose SHF_RO_AFTER_INIT through uapi (although that is where all the other SHF_* flags are defined) SHF_RO_AFTER_INIT would technically be used only internally in the kernel (i.e. module loader), and it'd also be considered a non-standard flag, using a bit from SHF_MASKOS (OS-specific range). What do you think? >If you could re-spin, that would be great. Once I figure out where to put SHF_RO_AFTER_INIT, I'll send a v2. Thanks for the review! Jessica > >> --- >> include/linux/module.h | 2 ++ >> include/uapi/linux/elf.h | 1 + >> kernel/module.c | 73 +++++++++++++++++++++++++++++++++++++++++------- >> 3 files changed, 66 insertions(+), 10 deletions(-) >> >> diff --git a/include/linux/module.h b/include/linux/module.h >> index f777164..a698d23 100644 >> --- a/include/linux/module.h >> +++ b/include/linux/module.h >> @@ -311,6 +311,8 @@ struct module_layout { >> unsigned int text_size; >> /* Size of RO section of the module (text+rodata) */ >> unsigned int ro_size; >> + /* Size of RO after init section */ >> + unsigned int ro_after_init_size; >> >> #ifdef CONFIG_MODULES_TREE_LOOKUP >> struct mod_tree_node mtn; >> diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h >> index cb4a72f..70b172ba 100644 >> --- a/include/uapi/linux/elf.h >> +++ b/include/uapi/linux/elf.h >> @@ -286,6 +286,7 @@ typedef struct elf64_phdr { >> #define SHF_ALLOC 0x2 >> #define SHF_EXECINSTR 0x4 >> #define SHF_RELA_LIVEPATCH 0x00100000 >> +#define SHF_RO_AFTER_INIT 0x00200000 >> #define SHF_MASKPROC 0xf0000000 >> >> /* special section indexes */ >> diff --git a/kernel/module.c b/kernel/module.c >> index 7f21ab2..055bf6f 100644 >> --- a/kernel/module.c >> +++ b/kernel/module.c >> @@ -1857,10 +1857,11 @@ static void mod_sysfs_teardown(struct module *mod) >> * from modification and any data from execution. >> * >> * General layout of module is: >> - * [text] [read-only-data] [writable data] >> - * text_size -----^ ^ ^ >> - * ro_size ------------------------| | >> - * size -------------------------------------------| >> + * [text] [read-only-data] [ro-after-init] [writable data] >> + * text_size -----^ ^ ^ ^ >> + * ro_size ------------------------| | | >> + * ro_after_init_size -----------------------------| | >> + * size -----------------------------------------------------------| >> * >> * These values are always page-aligned (as is base) >> */ >> @@ -1883,14 +1884,24 @@ static void frob_rodata(const struct module_layout *layout, >> (layout->ro_size - layout->text_size) >> PAGE_SHIFT); >> } >> >> +static void frob_ro_after_init(const struct module_layout *layout, >> + int (*set_memory)(unsigned long start, int num_pages)) >> +{ >> + BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); >> + BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); >> + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); >> + set_memory((unsigned long)layout->base + layout->ro_size, >> + (layout->ro_after_init_size - layout->ro_size) >> PAGE_SHIFT); >> +} >> + >> static void frob_writable_data(const struct module_layout *layout, >> int (*set_memory)(unsigned long start, int num_pages)) >> { >> BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1)); >> - BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1)); >> + BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1)); >> BUG_ON((unsigned long)layout->size & (PAGE_SIZE-1)); >> - set_memory((unsigned long)layout->base + layout->ro_size, >> - (layout->size - layout->ro_size) >> PAGE_SHIFT); >> + set_memory((unsigned long)layout->base + layout->ro_after_init_size, >> + (layout->size - layout->ro_after_init_size) >> PAGE_SHIFT); >> } >> >> /* livepatching wants to disable read-only so it can frob module. */ >> @@ -1898,6 +1909,7 @@ void module_disable_ro(const struct module *mod) >> { >> frob_text(&mod->core_layout, set_memory_rw); >> frob_rodata(&mod->core_layout, set_memory_rw); >> + frob_ro_after_init(&mod->core_layout, set_memory_rw); >> frob_text(&mod->init_layout, set_memory_rw); >> frob_rodata(&mod->init_layout, set_memory_rw); >> } >> @@ -1908,11 +1920,16 @@ void module_enable_ro(const struct module *mod) >> frob_rodata(&mod->core_layout, set_memory_ro); >> frob_text(&mod->init_layout, set_memory_ro); >> frob_rodata(&mod->init_layout, set_memory_ro); >> + >> + /* after init */ >> + if (mod->state == MODULE_STATE_LIVE) >> + frob_ro_after_init(&mod->core_layout, set_memory_ro); >> } >> >> static void module_enable_nx(const struct module *mod) >> { >> frob_rodata(&mod->core_layout, set_memory_nx); >> + frob_ro_after_init(&mod->core_layout, set_memory_nx); >> frob_writable_data(&mod->core_layout, set_memory_nx); >> frob_rodata(&mod->init_layout, set_memory_nx); >> frob_writable_data(&mod->init_layout, set_memory_nx); >> @@ -1921,6 +1938,7 @@ static void module_enable_nx(const struct module *mod) >> static void module_disable_nx(const struct module *mod) >> { >> frob_rodata(&mod->core_layout, set_memory_x); >> + frob_ro_after_init(&mod->core_layout, set_memory_x); >> frob_writable_data(&mod->core_layout, set_memory_x); >> frob_rodata(&mod->init_layout, set_memory_x); >> frob_writable_data(&mod->init_layout, set_memory_x); >> @@ -1963,6 +1981,8 @@ static void disable_ro_nx(const struct module_layout *layout) >> frob_text(layout, set_memory_rw); >> frob_rodata(layout, set_memory_rw); >> frob_rodata(layout, set_memory_x); >> + frob_ro_after_init(layout, set_memory_rw); >> + frob_ro_after_init(layout, set_memory_x); >> frob_writable_data(layout, set_memory_x); >> } >> >> @@ -2305,6 +2325,7 @@ static void layout_sections(struct module *mod, struct load_info *info) >> * finder in the two loops below */ >> { SHF_EXECINSTR | SHF_ALLOC, ARCH_SHF_SMALL }, >> { SHF_ALLOC, SHF_WRITE | ARCH_SHF_SMALL }, >> + { SHF_RO_AFTER_INIT | SHF_ALLOC, ARCH_SHF_SMALL }, >> { SHF_WRITE | SHF_ALLOC, ARCH_SHF_SMALL }, >> { ARCH_SHF_SMALL | SHF_ALLOC, 0 } >> }; >> @@ -2336,7 +2357,11 @@ static void layout_sections(struct module *mod, struct load_info *info) >> mod->core_layout.size = debug_align(mod->core_layout.size); >> mod->core_layout.ro_size = mod->core_layout.size; >> break; >> - case 3: /* whole core */ >> + case 2: /* RO after init */ >> + mod->core_layout.size = debug_align(mod->core_layout.size); >> + mod->core_layout.ro_after_init_size = mod->core_layout.size; >> + break; >> + case 4: /* whole core */ >> mod->core_layout.size = debug_align(mod->core_layout.size); >> break; >> } >> @@ -2366,7 +2391,14 @@ static void layout_sections(struct module *mod, struct load_info *info) >> mod->init_layout.size = debug_align(mod->init_layout.size); >> mod->init_layout.ro_size = mod->init_layout.size; >> break; >> - case 3: /* whole init */ >> + case 2: >> + /* >> + * RO after init doesn't apply to init_layout (only >> + * core_layout), so it just takes the value of ro_size. >> + */ >> + mod->init_layout.ro_after_init_size = mod->init_layout.ro_size; >> + break; >> + case 4: /* whole init */ >> mod->init_layout.size = debug_align(mod->init_layout.size); >> break; >> } >> @@ -3172,6 +3204,7 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) >> { >> /* Module within temporary copy. */ >> struct module *mod; >> + unsigned int ndx; >> int err; >> >> mod = setup_load_info(info, flags); >> @@ -3191,6 +3224,15 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) >> /* We will do a special allocation for per-cpu sections later. */ >> info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC; >> >> + /* >> + * Mark ro_after_init section with SHF_RO_AFTER_INIT so that >> + * layout_sections() can put it in the right place. >> + * Note: ro_after_init sections also have SHF_{WRITE,ALLOC} set. >> + */ >> + ndx = find_sec(info, ".data..ro_after_init"); >> + if (ndx) >> + info->sechdrs[ndx].sh_flags |= SHF_RO_AFTER_INIT; >> + >> /* Determine total sizes, and put offsets in sh_entsize. For now >> this is done generically; there doesn't appear to be any >> special cases for the architectures. */ >> @@ -3357,12 +3399,19 @@ static noinline int do_init_module(struct module *mod) >> /* Switch to core kallsyms now init is done: kallsyms may be walking! */ >> rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms); >> #endif >> + /* >> + * RO and NX regions should already be protected at this point, >> + * so the below module_enable_ro() call enables additional RO >> + * protection for the ro_after_init section only. >> + */ >> + module_enable_ro(mod); >> mod_tree_remove_init(mod); >> disable_ro_nx(&mod->init_layout); >> module_arch_freeing_init(mod); >> mod->init_layout.base = NULL; >> mod->init_layout.size = 0; >> mod->init_layout.ro_size = 0; >> + mod->init_layout.ro_after_init_size = 0; >> mod->init_layout.text_size = 0; >> /* >> * We want to free module_init, but be aware that kallsyms may be >> @@ -3454,7 +3503,11 @@ static int complete_formation(struct module *mod, struct load_info *info) >> /* This relies on module_mutex for list integrity. */ >> module_bug_finalize(info->hdr, info->sechdrs, mod); >> >> - /* Set RO and NX regions */ >> + /* >> + * Set RO and NX regions. Since module is not LIVE yet, >> + * the ro_after_init section will remain RW until after >> + * do_one_initcall(). >> + */ >> module_enable_ro(mod); >> module_enable_nx(mod); >> >> -- >> 2.4.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: modules: add ro_after_init support 2016-06-29 21:27 ` Jessica Yu @ 2016-06-30 4:56 ` Rusty Russell 2016-07-21 23:03 ` Kees Cook 0 siblings, 1 reply; 9+ messages in thread From: Rusty Russell @ 2016-06-30 4:56 UTC (permalink / raw) To: Jessica Yu; +Cc: Kees Cook, linux-api, linux-kernel Jessica Yu <jeyu@redhat.com> writes: > +++ Rusty Russell [29/06/16 10:38 +0930]: >>Jessica Yu <jeyu@redhat.com> writes: >>> Add ro_after_init support for modules by adding a new page-aligned section >>> in the module layout (after rodata) for ro_after_init data and enabling RO >>> protection for that section after module init runs. >>> >>> Signed-off-by: Jessica Yu <jeyu@redhat.com> >> >>I would prefer a "bool after_init" flag to module_enable_ro(). It's >>more explicit. > > Sure thing, I was just initially worried about the > module_{enable,disable}_ro() asymmetry. :) Yes, but I think compile-time-analyzable behaviour beats runtime-analyzable behaviour for clarity. >>Exposing the flags via uapi looks like a wart, but it's kind of a >>feature, since we don't *unset* it in any section; userspace may want to >>know about it. > > Hm, I'm still unsure about this. I'm starting to think it might be a > bit overkill to expose SHF_RO_AFTER_INIT through uapi (although that > is where all the other SHF_* flags are defined) SHF_RO_AFTER_INIT > would technically be used only internally in the kernel (i.e. module > loader), and it'd also be considered a non-standard flag, using a bit > from SHF_MASKOS (OS-specific range). What do you think? Some arch *could* use it by setting the flag in a section in their module I think; we don't stop them. Since the other flags are there, I'd leave it. We don't expose the flags via sysfs, though, so that's the only exposure. Thanks! Rusty. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: modules: add ro_after_init support 2016-06-30 4:56 ` Rusty Russell @ 2016-07-21 23:03 ` Kees Cook 2016-07-21 23:11 ` Jessica Yu 0 siblings, 1 reply; 9+ messages in thread From: Kees Cook @ 2016-07-21 23:03 UTC (permalink / raw) To: Rusty Russell; +Cc: Jessica Yu, Linux API, LKML On Wed, Jun 29, 2016 at 9:56 PM, Rusty Russell <rusty@rustcorp.com.au> wrote: > Jessica Yu <jeyu@redhat.com> writes: >> +++ Rusty Russell [29/06/16 10:38 +0930]: >>>Jessica Yu <jeyu@redhat.com> writes: >>>> Add ro_after_init support for modules by adding a new page-aligned section >>>> in the module layout (after rodata) for ro_after_init data and enabling RO >>>> protection for that section after module init runs. >>>> >>>> Signed-off-by: Jessica Yu <jeyu@redhat.com> >>> >>>I would prefer a "bool after_init" flag to module_enable_ro(). It's >>>more explicit. >> >> Sure thing, I was just initially worried about the >> module_{enable,disable}_ro() asymmetry. :) > > Yes, but I think compile-time-analyzable behaviour beats > runtime-analyzable behaviour for clarity. > >>>Exposing the flags via uapi looks like a wart, but it's kind of a >>>feature, since we don't *unset* it in any section; userspace may want to >>>know about it. >> >> Hm, I'm still unsure about this. I'm starting to think it might be a >> bit overkill to expose SHF_RO_AFTER_INIT through uapi (although that >> is where all the other SHF_* flags are defined) SHF_RO_AFTER_INIT >> would technically be used only internally in the kernel (i.e. module >> loader), and it'd also be considered a non-standard flag, using a bit >> from SHF_MASKOS (OS-specific range). What do you think? > > Some arch *could* use it by setting the flag in a section in their > module I think; we don't stop them. Since the other flags are there, > I'd leave it. > > We don't expose the flags via sysfs, though, so that's the only > exposure. What's the state of this series? I'd love it if the functionality could land for v4.8... -Kees -- Kees Cook Brillo & Chrome OS Security ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: modules: add ro_after_init support 2016-07-21 23:03 ` Kees Cook @ 2016-07-21 23:11 ` Jessica Yu 0 siblings, 0 replies; 9+ messages in thread From: Jessica Yu @ 2016-07-21 23:11 UTC (permalink / raw) To: Kees Cook; +Cc: Rusty Russell, Linux API, LKML +++ Kees Cook [21/07/16 16:03 -0700]: >On Wed, Jun 29, 2016 at 9:56 PM, Rusty Russell <rusty@rustcorp.com.au> wrote: >> Jessica Yu <jeyu@redhat.com> writes: >>> +++ Rusty Russell [29/06/16 10:38 +0930]: >>>>Jessica Yu <jeyu@redhat.com> writes: >>>>> Add ro_after_init support for modules by adding a new page-aligned section >>>>> in the module layout (after rodata) for ro_after_init data and enabling RO >>>>> protection for that section after module init runs. >>>>> >>>>> Signed-off-by: Jessica Yu <jeyu@redhat.com> >>>> >>>>I would prefer a "bool after_init" flag to module_enable_ro(). It's >>>>more explicit. >>> >>> Sure thing, I was just initially worried about the >>> module_{enable,disable}_ro() asymmetry. :) >> >> Yes, but I think compile-time-analyzable behaviour beats >> runtime-analyzable behaviour for clarity. >> >>>>Exposing the flags via uapi looks like a wart, but it's kind of a >>>>feature, since we don't *unset* it in any section; userspace may want to >>>>know about it. >>> >>> Hm, I'm still unsure about this. I'm starting to think it might be a >>> bit overkill to expose SHF_RO_AFTER_INIT through uapi (although that >>> is where all the other SHF_* flags are defined) SHF_RO_AFTER_INIT >>> would technically be used only internally in the kernel (i.e. module >>> loader), and it'd also be considered a non-standard flag, using a bit >>> from SHF_MASKOS (OS-specific range). What do you think? >> >> Some arch *could* use it by setting the flag in a section in their >> module I think; we don't stop them. Since the other flags are there, >> I'd leave it. >> >> We don't expose the flags via sysfs, though, so that's the only >> exposure. > >What's the state of this series? I'd love it if the functionality >could land for v4.8... > Hi Kees, Sorry for the delay! Have been busier than usual lately. I'll be able to get v2 out tomorrow. Thanks! Jessica ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-07-25 18:04 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-07-25 9:25 [PATCH v2 0/1] Add ro_after_init support for modules Jessica Yu 2016-07-25 9:25 ` [PATCH v2 1/1] modules: add ro_after_init support Jessica Yu 2016-07-25 10:01 ` Jessica Yu 2016-07-25 18:04 ` [PATCH v2 1/1] " Kees Cook -- strict thread matches above, loose matches on Subject: below -- 2016-06-14 0:13 [PATCH 0/1] Add ro_after_init support for modules Jessica Yu 2016-06-14 0:13 ` [PATCH 1/1] modules: add ro_after_init support Jessica Yu 2016-06-14 21:33 ` Kees Cook 2016-06-14 23:53 ` Jessica Yu 2016-06-29 1:08 ` [PATCH 1/1] " Rusty Russell 2016-06-29 21:27 ` Jessica Yu 2016-06-30 4:56 ` Rusty Russell 2016-07-21 23:03 ` Kees Cook 2016-07-21 23:11 ` Jessica Yu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).