Messages in this thread | | | Date | Thu, 14 Jan 2021 22:15:42 +0000 | From | Frank van der Linden <> | Subject | Re: [PATCH v2] module: harden ELF info handling |
| |
On Wed, Jan 13, 2021 at 03:48:42PM +0100, Jessica Yu wrote: > > [...] > > + err = validate_section_offset(info, shdr); > > + if (err < 0) { > > + pr_err("Invalid ELF section in module (section %u type %u)\n", > > + i, shdr->sh_type); > > + return err; > > + } > > + > > + if (shdr->sh_flags & SHF_ALLOC) { > > + if (shdr->sh_name >= strhdr->sh_size) { > > + pr_err("Invalid ELF section name in module (section num %u type %u)\n", > > Small nit: Maybe remove "num", to be consistent with the other pr_err() above.
Sure, will do.
[...] > > diff --git a/kernel/module_signing.c b/kernel/module_signing.c > > index 9d9fc678c91d..9a057c5d1d4d 100644 > > --- a/kernel/module_signing.c > > +++ b/kernel/module_signing.c > > @@ -30,7 +30,7 @@ int mod_verify_sig(const void *mod, struct load_info *info) > > > > memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms)); > > > > - ret = mod_check_sig(&ms, modlen, info->name); > > + ret = mod_check_sig(&ms, modlen, info->name ?: "module"); > > Since info->name is not expected to be valid anymore, as we're back to > calling mod_sig_check() first thing, perhaps just stick with > "module"? Or was there another reason for checking info->name here?
It's mainly "what if mod_check_sig() is called differently in the future?", but you are of course right: info->name will be NULL here. I'll change it to just "module".
Thanks,
- Frank
| |