Problem: it's hard to avoid an init routine stumbling over a request_module these days. And it's not clear it's always a bad idea: for example, a module like kvm with dynamic dependencies on kvm-intel or kvm-amd would be neater if it could simply request_module the right one. In this particular case, it's libcrc32c: libcrc32c_mod_init crypto_alloc_shash crypto_alloc_tfm crypto_find_alg crypto_alg_mod_lookup crypto_larval_lookup request_module If another module is waiting inside resolve_symbol() for libcrc32c to finish initializing (ie. bne2 depends on libcrc32c) then it does so holding the module lock, and our request_module() can't make progress until that is released. Waiting inside resolve_symbol() without the lock isn't all that hard: we just need to pass the -EBUSY up the call chain so we can sleep where we don't hold the lock. Error reporting is a bit trickier: we need to copy the name of the unfinished module before releasing the lock. Other notes: 1) This also fixes a theoretical issue where a weak dependency would allow symbol version mismatches to be ignored. 2) We rename use_module to ref_module to make life easier for the only external user (the out-of-tree ksplice patches).
Unified · Split
| @@ -582,33 +582,26 @@ static int add_module_usage(struct module *a, struct module *b) · 581 unmodified lines | ||
| 582 | 582 | } |
| 583 | 583 | |
| 584 | 584 | /* Module a uses b: caller needs module_mutex() */ |
| 585 | −int use_module(struct module *a, struct module *b) | |
| 585 | +int ref_module(struct module *a, struct module *b) | |
| 586 | 586 | { |
| 587 | 587 | int err; |
| 588 | 588 | |
| 589 | − if (b == NULL || already_uses(a, b)) return 1; | |
| 590 | − | |
| 591 | − /* If we're interrupted or time out, we fail. */ | |
| 592 | − if (wait_event_interruptible_timeout( | |
| 593 | − module_wq, (err = strong_try_module_get(b)) != -EBUSY, | |
| 594 | − 30 * HZ) <= 0) { | |
| 595 | − printk("%s: gave up waiting for init of module %s.\n", | |
| 596 | − a->name, b->name); | |
| 589 | + if (b == NULL || already_uses(a, b)) | |
| 597 | 590 | return 0; |
| 598 | − } | |
| 599 | 591 | |
| 600 | − /* If strong_try_module_get() returned a different error, we fail. */ | |
| 592 | + /* If module isn't available, we fail. */ | |
| 593 | + err = strong_try_module_get(b); | |
| 601 | 594 | if (err) |
| 602 | − return 0; | |
| 595 | + return err; | |
| 603 | 596 | |
| 604 | 597 | err = add_module_usage(a, b); |
| 605 | 598 | if (err) { |
| 606 | 599 | module_put(b); |
| 607 | − return 0; | |
| 600 | + return err; | |
| 608 | 601 | } |
| 609 | − return 1; | |
| 602 | + return 0; | |
| 610 | 603 | } |
| 611 | −EXPORT_SYMBOL_GPL(use_module); | |
| 604 | +EXPORT_SYMBOL_GPL(ref_module); | |
| 612 | 605 | |
| 613 | 606 | /* Clear the unload stuff of the module. */ |
| 614 | 607 | static void module_unload_free(struct module *mod) |
| @@ -893,11 +886,11 @@ static inline void module_unload_free(struct module *mod) · 278 unmodified lines | ||
| 893 | 886 | { |
| 894 | 887 | } |
| 895 | 888 | |
| 896 | −int use_module(struct module *a, struct module *b) | |
| 889 | +int ref_module(struct module *a, struct module *b) | |
| 897 | 890 | { |
| 898 | − return strong_try_module_get(b) == 0; | |
| 891 | + return strong_try_module_get(b); | |
| 899 | 892 | } |
| 900 | −EXPORT_SYMBOL_GPL(use_module); | |
| 893 | +EXPORT_SYMBOL_GPL(ref_module); | |
| 901 | 894 | |
| 902 | 895 | static inline void module_unload_init(struct module *mod) |
| 903 | 896 | { |
| @@ -1062,26 +1055,58 @@ static inline int same_magic(const char *amagic, const char *bmagic, · 158 unmodified lines | ||
| 1062 | 1055 | static const struct kernel_symbol *resolve_symbol(Elf_Shdr *sechdrs, |
| 1063 | 1056 | unsigned int versindex, |
| 1064 | 1057 | const char *name, |
| 1065 | − struct module *mod) | |
| 1058 | + struct module *mod, | |
| 1059 | + char ownername[]) | |
| 1066 | 1060 | { |
| 1067 | 1061 | struct module *owner; |
| 1068 | 1062 | const struct kernel_symbol *sym; |
| 1069 | 1063 | const unsigned long *crc; |
| 1064 | + int err; | |
| 1070 | 1065 | |
| 1071 | 1066 | mutex_lock(&module_mutex); |
| 1072 | 1067 | sym = find_symbol(name, &owner, &crc, |
| 1073 | 1068 | !(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)), true); |
| 1074 | − /* use_module can fail due to OOM, | |
| 1075 | − or module initialization or unloading */ | |
| 1076 | − if (sym) { | |
| 1077 | − if (!check_version(sechdrs, versindex, name, mod, crc, owner) | |
| 1078 | − || !use_module(mod, owner)) | |
| 1079 | − sym = NULL; | |
| 1069 | + if (!sym) | |
| 1070 | + goto unlock; | |
| 1071 | + | |
| 1072 | + if (!check_version(sechdrs, versindex, name, mod, crc, owner)) { | |
| 1073 | + sym = ERR_PTR(-EINVAL); | |
| 1074 | + goto getname; | |
| 1080 | 1075 | } |
| 1076 | + | |
| 1077 | + err = ref_module(mod, owner); | |
| 1078 | + if (err) { | |
| 1079 | + sym = ERR_PTR(err); | |
| 1080 | + goto getname; | |
| 1081 | + } | |
| 1082 | + | |
| 1083 | +getname: | |
| 1084 | + /* We must make copy under the lock if we failed to get ref. */ | |
| 1085 | + strncpy(ownername, module_name(owner), MODULE_NAME_LEN); | |
| 1086 | +unlock: | |
| 1081 | 1087 | mutex_unlock(&module_mutex); |
| 1082 | 1088 | return sym; |
| 1083 | 1089 | } |
| 1084 | 1090 | |
| 1091 | +static const struct kernel_symbol *resolve_symbol_wait(Elf_Shdr *sechdrs, | |
| 1092 | + unsigned int versindex, | |
| 1093 | + const char *name, | |
| 1094 | + struct module *mod) | |
| 1095 | +{ | |
| 1096 | + const struct kernel_symbol *ksym; | |
| 1097 | + char ownername[MODULE_NAME_LEN]; | |
| 1098 | + | |
| 1099 | + if (wait_event_interruptible_timeout(module_wq, | |
| 1100 | + !IS_ERR(ksym = resolve_symbol(sechdrs, versindex, name, | |
| 1101 | + mod, ownername)) || | |
| 1102 | + PTR_ERR(ksym) != -EBUSY, | |
| 1103 | + 30 * HZ) <= 0) { | |
| 1104 | + printk(KERN_WARNING "%s: gave up waiting for init of module %s.\n", | |
| 1105 | + mod->name, ownername); | |
| 1106 | + } | |
| 1107 | + return ksym; | |
| 1108 | +} | |
| 1109 | + | |
| 1085 | 1110 | /* |
| 1086 | 1111 | * /sys/module/foo/sections stuff |
| 1087 | 1112 | * J. Corbet <corbet@lwn.net> |
| @@ -1638,21 +1663,23 @@ static int simplify_symbols(Elf_Shdr *sechdrs, · 550 unmodified lines | ||
| 1638 | 1663 | break; |
| 1639 | 1664 | |
| 1640 | 1665 | case SHN_UNDEF: |
| 1641 | − ksym = resolve_symbol(sechdrs, versindex, | |
| 1642 | − strtab + sym[i].st_name, mod); | |
| 1666 | + ksym = resolve_symbol_wait(sechdrs, versindex, | |
| 1667 | + strtab + sym[i].st_name, | |
| 1668 | + mod); | |
| 1643 | 1669 | /* Ok if resolved. */ |
| 1644 | − if (ksym) { | |
| 1670 | + if (ksym && !IS_ERR(ksym)) { | |
| 1645 | 1671 | sym[i].st_value = ksym->value; |
| 1646 | 1672 | break; |
| 1647 | 1673 | } |
| 1648 | 1674 | |
| 1649 | 1675 | /* Ok if weak. */ |
| 1650 | − if (ELF_ST_BIND(sym[i].st_info) == STB_WEAK) | |
| 1676 | + if (!ksym && ELF_ST_BIND(sym[i].st_info) == STB_WEAK) | |
| 1651 | 1677 | break; |
| 1652 | 1678 | |
| 1653 | − printk(KERN_WARNING "%s: Unknown symbol %s\n", | |
| 1654 | − mod->name, strtab + sym[i].st_name); | |
| 1655 | − ret = -ENOENT; | |
| 1679 | + printk(KERN_WARNING "%s: Unknown symbol %s (err %li)\n", | |
| 1680 | + mod->name, strtab + sym[i].st_name, | |
| 1681 | + PTR_ERR(ksym)); | |
| 1682 | + ret = PTR_ERR(ksym) ?: -ENOENT; | |
| 1656 | 1683 | break; |
| 1657 | 1684 | |
| 1658 | 1685 | default: |