History

module: harden ELF info handling

5fdc7db644 ("module: setup load info before module_sig_check()") moved the ELF setup, so that it was done before the signature check. This made the module name available to signature error messages. However, the checks for ELF correctness in setup_load_info are not sufficient to prevent bad memory references due to corrupted offset fields, indices, etc. So, there's a regression in behavior here: a corrupt and unsigned (or badly signed) module, which might previously have been rejected immediately, can now cause an oops/crash. Harden ELF handling for module loading by doing the following: - Move the signature check back up so that it comes before ELF initialization. It's best to do the signature check to see if we can trust the module, before using the ELF structures inside it. This also makes checks against info->len more accurate again, as this field will be reduced by the length of the signature in mod_check_sig(). The module name is now once again not available for error messages during the signature check, but that seems like a fair tradeoff. - Check if sections have offset / size fields that at least don't exceed the length of the module. - Check if sections have section name offsets that don't fall outside the section name table. - Add a few other sanity checks against invalid section indices, etc. This is not an exhaustive consistency check, but the idea is to at least get through the signature and blacklist checks without crashing because of corrupted ELF info, and to error out gracefully for most issues that would have caused problems later on.

Frank van der Linden committed 5 years ago

Commit ec2a2951 parent ebfac7b3 files · +126 −21Browse files

Unified · Split

3 files
  1. kernel/module.c +124 −19
  2. kernel/module_signature.c +1 −1
  3. kernel/module_signing.c +1 −1
kernel/module.c+124−19
@@ -2981,7 +2981,7 @@ static int module_sig_check(struct load_info *info, int flags) · 2980 unmodified lines
29812981 }
29822982
29832983 if (is_module_sig_enforced()) {
2984− pr_notice("%s: loading of %s is rejected\n", info->name, reason);
2984+ pr_notice("Loading of %s is rejected\n", reason);
29852985 return -EKEYREJECTED;
29862986 }
29872987
@@ -2994,9 +2994,33 @@ static int module_sig_check(struct load_info *info, int flags) · 6 unmodified lines
29942994 }
29952995 #endif /* !CONFIG_MODULE_SIG */
29962996
2997−/* Sanity checks against invalid binaries, wrong arch, weird elf version. */
2998−static int elf_header_check(struct load_info *info)
2997+static int validate_section_offset(struct load_info *info, Elf_Shdr *shdr)
29992998 {
2999+ unsigned long secend;
3000+
3001+ /*
3002+ * Check for both overflow and offset/size being
3003+ * too large.
3004+ */
3005+ secend = shdr->sh_offset + shdr->sh_size;
3006+ if (secend < shdr->sh_offset || secend > info->len)
3007+ return -ENOEXEC;
3008+
3009+ return 0;
3010+}
3011+
3012+/*
3013+ * Sanity checks against invalid binaries, wrong arch, weird elf version.
3014+ *
3015+ * Also do basic validity checks against section offsets and sizes, the
3016+ * section name string table, and the indices used for it (sh_name).
3017+ */
3018+static int elf_validity_check(struct load_info *info)
3019+{
3020+ unsigned int i;
3021+ Elf_Shdr *shdr, *strhdr;
3022+ int err;
3023+
30003024 if (info->len < sizeof(*(info->hdr)))
30013025 return -ENOEXEC;
30023026
@@ -3006,11 +3030,78 @@ static int elf_header_check(struct load_info *info) · 3 unmodified lines
30063030 || info->hdr->e_shentsize != sizeof(Elf_Shdr))
30073031 return -ENOEXEC;
30083032
3033+ /*
3034+ * e_shnum is 16 bits, and sizeof(Elf_Shdr) is
3035+ * known and small. So e_shnum * sizeof(Elf_Shdr)
3036+ * will not overflow unsigned long on any platform.
3037+ */
30093038 if (info->hdr->e_shoff >= info->len
30103039 || (info->hdr->e_shnum * sizeof(Elf_Shdr) >
30113040 info->len - info->hdr->e_shoff))
30123041 return -ENOEXEC;
30133042
3043+ info->sechdrs = (void *)info->hdr + info->hdr->e_shoff;
3044+
3045+ /*
3046+ * Verify if the section name table index is valid.
3047+ */
3048+ if (info->hdr->e_shstrndx == SHN_UNDEF
3049+ || info->hdr->e_shstrndx >= info->hdr->e_shnum)
3050+ return -ENOEXEC;
3051+
3052+ strhdr = &info->sechdrs[info->hdr->e_shstrndx];
3053+ err = validate_section_offset(info, strhdr);
3054+ if (err < 0)
3055+ return err;
3056+
3057+ /*
3058+ * The section name table must be NUL-terminated, as required
3059+ * by the spec. This makes strcmp and pr_* calls that access
3060+ * strings in the section safe.
3061+ */
3062+ info->secstrings = (void *)info->hdr + strhdr->sh_offset;
3063+ if (info->secstrings[strhdr->sh_size - 1] != '\0')
3064+ return -ENOEXEC;
3065+
3066+ /*
3067+ * The code assumes that section 0 has a length of zero and
3068+ * an addr of zero, so check for it.
3069+ */
3070+ if (info->sechdrs[0].sh_type != SHT_NULL
3071+ || info->sechdrs[0].sh_size != 0
3072+ || info->sechdrs[0].sh_addr != 0)
3073+ return -ENOEXEC;
3074+
3075+ for (i = 1; i < info->hdr->e_shnum; i++) {
3076+ shdr = &info->sechdrs[i];
3077+ switch (shdr->sh_type) {
3078+ case SHT_NULL:
3079+ case SHT_NOBITS:
3080+ continue;
3081+ case SHT_SYMTAB:
3082+ if (shdr->sh_link == SHN_UNDEF
3083+ || shdr->sh_link >= info->hdr->e_shnum)
3084+ return -ENOEXEC;
3085+ fallthrough;
3086+ default:
3087+ err = validate_section_offset(info, shdr);
3088+ if (err < 0) {
3089+ pr_err("Invalid ELF section in module (section %u type %u)\n",
3090+ i, shdr->sh_type);
3091+ return err;
3092+ }
3093+
3094+ if (shdr->sh_flags & SHF_ALLOC) {
3095+ if (shdr->sh_name >= strhdr->sh_size) {
3096+ pr_err("Invalid ELF section name in module (section %u type %u)\n",
3097+ i, shdr->sh_type);
3098+ return -ENOEXEC;
3099+ }
3100+ }
3101+ break;
3102+ }
3103+ }
3104+
30143105 return 0;
30153106 }
30163107
@@ -3112,11 +3203,6 @@ static int rewrite_section_headers(struct load_info *info, int flags) · 95 unmodified lines
31123203
31133204 for (i = 1; i < info->hdr->e_shnum; i++) {
31143205 Elf_Shdr *shdr = &info->sechdrs[i];
3115− if (shdr->sh_type != SHT_NOBITS
3116− && info->len < shdr->sh_offset + shdr->sh_size) {
3117− pr_err("Module len %lu truncated\n", info->len);
3118− return -ENOEXEC;
3119− }
31203206
31213207 /*
31223208 * Mark all sections sh_addr with their address in the
@@ -3150,11 +3236,6 @@ static int setup_load_info(struct load_info *info, int flags) · 27 unmodified lines
31503236 {
31513237 unsigned int i;
31523238
3153− /* Set up the convenience variables */
3154− info->sechdrs = (void *)info->hdr + info->hdr->e_shoff;
3155− info->secstrings = (void *)info->hdr
3156− + info->sechdrs[info->hdr->e_shstrndx].sh_offset;
3157−
31583239 /* Try to find a name early so we can log errors with a module name */
31593240 info->index.info = find_sec(info, ".modinfo");
31603241 if (info->index.info)
@@ -3911,26 +3992,50 @@ static int load_module(struct load_info *info, const char __user *uargs, · 750 unmodified lines
39113992 long err = 0;
39123993 char *after_dashes;
39133994
3914− err = elf_header_check(info);
3995+ /*
3996+ * Do the signature check (if any) first. All that
3997+ * the signature check needs is info->len, it does
3998+ * not need any of the section info. That can be
3999+ * set up later. This will minimize the chances
4000+ * of a corrupt module causing problems before
4001+ * we even get to the signature check.
4002+ *
4003+ * The check will also adjust info->len by stripping
4004+ * off the sig length at the end of the module, making
4005+ * checks against info->len more correct.
4006+ */
4007+ err = module_sig_check(info, flags);
4008+ if (err)
4009+ goto free_copy;
4010+
4011+ /*
4012+ * Do basic sanity checks against the ELF header and
4013+ * sections.
4014+ */
4015+ err = elf_validity_check(info);
39154016 if (err) {
3916− pr_err("Module has invalid ELF header\n");
4017+ pr_err("Module has invalid ELF structures\n");
39174018 goto free_copy;
39184019 }
39194020
4021+ /*
4022+ * Everything checks out, so set up the section info
4023+ * in the info structure.
4024+ */
39204025 err = setup_load_info(info, flags);
39214026 if (err)
39224027 goto free_copy;
39234028
4029+ /*
4030+ * Now that we know we have the correct module name, check
4031+ * if it's blacklisted.
4032+ */
39244033 if (blacklisted(info->name)) {
39254034 err = -EPERM;
39264035 pr_err("Module %s is blacklisted\n", info->name);
39274036 goto free_copy;
39284037 }
39294038
3930− err = module_sig_check(info, flags);
3931− if (err)
3932− goto free_copy;
3933−
39344039 err = rewrite_section_headers(info, flags);
39354040 if (err)
39364041 goto free_copy;
kernel/module_signature.c+1−1
@@ -25,7 +25,7 @@ int mod_check_sig(const struct module_signature *ms, size_t file_len, · 24 unmodified lines
2525 return -EBADMSG;
2626
2727 if (ms->id_type != PKEY_ID_PKCS7) {
28− pr_err("%s: Module is not signed with expected PKCS#7 message\n",
28+ pr_err("%s: not signed with expected PKCS#7 message\n",
2929 name);
3030 return -ENOPKG;
3131 }
kernel/module_signing.c+1−1
@@ -30,7 +30,7 @@ int mod_verify_sig(const void *mod, struct load_info *info) · 29 unmodified lines
3030
3131 memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
3232
33− ret = mod_check_sig(&ms, modlen, info->name);
33+ ret = mod_check_sig(&ms, modlen, "module");
3434 if (ret)
3535 return ret;
3636

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t