Messages in this thread | | | Date | Wed, 8 Apr 2020 17:57:19 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v2] module: Harden STRICT_MODULE_RWX |
| |
On Wed, Apr 08, 2020 at 05:32:49PM +0200, Jessica Yu wrote: > +++ Peter Zijlstra [03/04/20 19:13 +0200]: > > > > We're very close to enforcing W^X memory, refuse to load modules that > > violate this principle per construction. > > > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > --- > > kernel/module.c | 25 +++++++++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > > > --- a/kernel/module.c > > +++ b/kernel/module.c > > @@ -2044,9 +2044,29 @@ static void module_enable_x(const struct > > frob_text(&mod->core_layout, set_memory_x); > > frob_text(&mod->init_layout, set_memory_x); > > } > > + > > +static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs, > > + char *secstrings, struct module *mod) > > +{ > > + const unsigned long shf_wx = SHF_WRITE|SHF_EXECINSTR; > > + int i; > > + > > + for (i = 0; i < hdr->e_shnum; i++) { > > + if ((sechdrs[i].sh_flags & shf_wx) == shf_wx) > > + return -ENOEXEC; > > + } > > + > > + return 0; > > +} > > Just to clarify, did we want to enforce this only when > CONFIG_STRICT_MODULE_RWX=y? Because here it's still in the > CONFIG_ARCH_HAS_STRICT_MODULE_RWX block. > > Unfortunately, when we add module_enforce_rwx_sections() in the > CONFIG_STRICT_MODULE_RWX block, we'll need two empty stubs, one for > !CONFIG_ARCH_HAS_STRICT_MODULE_RWX and one for !CONFIG_STRICT_MODULE_RWX. > > This is because the CONFIG_STRICT_MODULE_RWX block is currently nested > within ARCH_HAS_STRICT_MODULE_RWX :/
Yeah, so the primary reason it's under that ARCH_HAS thing is indeed the mess and the extra stub required (I'm a lazy sod at times).
I then rationalized this decision to myself that having it under ARCH_HAS give a more consistent module loading behaviour.
But I really don't care too much, my most my .config's have CONFIG_MODULE=n, and the ones that do not very much have the STRICT_RWX set.
Put it where you think it's best.
Thanks!
| |