Remove the livepatch module notifier in favor of directly enabling and disabling patches to modules in the module loader. Hard-coding the function calls ensures that ftrace_module_enable() is run before klp_module_coming() during module load, and that klp_module_going() is run before ftrace_release_mod() during module unload. This way, ftrace and livepatch code is run in the correct order during the module load/unload sequence without dependence on the module notifier call chain.
Showing the changes to kernel/livepatch (1 of 3 files). Show all
Unified · Split
| @@ -99,12 +99,12 @@ static void klp_find_object_module(struct klp_object *obj) · 98 unmodified lines | ||
| 99 | 99 | /* |
| 100 | 100 | * We do not want to block removal of patched modules and therefore |
| 101 | 101 | * we do not take a reference here. The patches are removed by |
| 102 | − * a going module handler instead. | |
| 102 | + * klp_module_going() instead. | |
| 103 | 103 | */ |
| 104 | 104 | mod = find_module(obj->name); |
| 105 | 105 | /* |
| 106 | − * Do not mess work of the module coming and going notifiers. | |
| 107 | − * Note that the patch might still be needed before the going handler | |
| 106 | + * Do not mess work of klp_module_coming() and klp_module_going(). | |
| 107 | + * Note that the patch might still be needed before klp_module_going() | |
| 108 | 108 | * is called. Module functions can be called even in the GOING state |
| 109 | 109 | * until mod->exit() finishes. This is especially important for |
| 110 | 110 | * patches that modify semantic of the functions. |
| @@ -866,103 +866,108 @@ int klp_register_patch(struct klp_patch *patch) · 755 unmodified lines | ||
| 866 | 866 | } |
| 867 | 867 | EXPORT_SYMBOL_GPL(klp_register_patch); |
| 868 | 868 | |
| 869 | −static int klp_module_notify_coming(struct klp_patch *patch, | |
| 870 | − struct klp_object *obj) | |
| 869 | +int klp_module_coming(struct module *mod) | |
| 871 | 870 | { |
| 872 | − struct module *pmod = patch->mod; | |
| 873 | − struct module *mod = obj->mod; | |
| 874 | 871 | int ret; |
| 872 | + struct klp_patch *patch; | |
| 873 | + struct klp_object *obj; | |
| 875 | 874 | |
| 876 | − ret = klp_init_object_loaded(patch, obj); | |
| 877 | − if (ret) { | |
| 878 | − pr_warn("failed to initialize patch '%s' for module '%s' (%d)\n", | |
| 879 | − pmod->name, mod->name, ret); | |
| 880 | − return ret; | |
| 881 | − } | |
| 875 | + if (WARN_ON(mod->state != MODULE_STATE_COMING)) | |
| 876 | + return -EINVAL; | |
| 882 | 877 | |
| 883 | − if (patch->state == KLP_DISABLED) | |
| 884 | − return 0; | |
| 878 | + mutex_lock(&klp_mutex); | |
| 879 | + /* | |
| 880 | + * Each module has to know that klp_module_coming() | |
| 881 | + * has been called. We never know what module will | |
| 882 | + * get patched by a new patch. | |
| 883 | + */ | |
| 884 | + mod->klp_alive = true; | |
| 885 | 885 | |
| 886 | − pr_notice("applying patch '%s' to loading module '%s'\n", | |
| 887 | − pmod->name, mod->name); | |
| 886 | + list_for_each_entry(patch, &klp_patches, list) { | |
| 887 | + klp_for_each_object(patch, obj) { | |
| 888 | + if (!klp_is_module(obj) || strcmp(obj->name, mod->name)) | |
| 889 | + continue; | |
| 888 | 890 | |
| 889 | − ret = klp_enable_object(obj); | |
| 890 | − if (ret) | |
| 891 | − pr_warn("failed to apply patch '%s' to module '%s' (%d)\n", | |
| 892 | − pmod->name, mod->name, ret); | |
| 893 | − return ret; | |
| 894 | −} | |
| 891 | + obj->mod = mod; | |
| 895 | 892 | |
| 896 | −static void klp_module_notify_going(struct klp_patch *patch, | |
| 897 | − struct klp_object *obj) | |
| 898 | −{ | |
| 899 | − struct module *pmod = patch->mod; | |
| 900 | − struct module *mod = obj->mod; | |
| 893 | + ret = klp_init_object_loaded(patch, obj); | |
| 894 | + if (ret) { | |
| 895 | + pr_warn("failed to initialize patch '%s' for module '%s' (%d)\n", | |
| 896 | + patch->mod->name, obj->mod->name, ret); | |
| 897 | + goto err; | |
| 898 | + } | |
| 901 | 899 | |
| 902 | − if (patch->state == KLP_DISABLED) | |
| 903 | − goto disabled; | |
| 900 | + if (patch->state == KLP_DISABLED) | |
| 901 | + break; | |
| 902 | + | |
| 903 | + pr_notice("applying patch '%s' to loading module '%s'\n", | |
| 904 | + patch->mod->name, obj->mod->name); | |
| 905 | + | |
| 906 | + ret = klp_enable_object(obj); | |
| 907 | + if (ret) { | |
| 908 | + pr_warn("failed to apply patch '%s' to module '%s' (%d)\n", | |
| 909 | + patch->mod->name, obj->mod->name, ret); | |
| 910 | + goto err; | |
| 911 | + } | |
| 912 | + | |
| 913 | + break; | |
| 914 | + } | |
| 915 | + } | |
| 904 | 916 | |
| 905 | − pr_notice("reverting patch '%s' on unloading module '%s'\n", | |
| 906 | − pmod->name, mod->name); | |
| 917 | + mutex_unlock(&klp_mutex); | |
| 907 | 918 | |
| 908 | − klp_disable_object(obj); | |
| 919 | + return 0; | |
| 909 | 920 | |
| 910 | −disabled: | |
| 921 | +err: | |
| 922 | + /* | |
| 923 | + * If a patch is unsuccessfully applied, return | |
| 924 | + * error to the module loader. | |
| 925 | + */ | |
| 926 | + pr_warn("patch '%s' failed for module '%s', refusing to load module '%s'\n", | |
| 927 | + patch->mod->name, obj->mod->name, obj->mod->name); | |
| 928 | + mod->klp_alive = false; | |
| 911 | 929 | klp_free_object_loaded(obj); |
| 930 | + mutex_unlock(&klp_mutex); | |
| 931 | + | |
| 932 | + return ret; | |
| 912 | 933 | } |
| 913 | 934 | |
| 914 | −static int klp_module_notify(struct notifier_block *nb, unsigned long action, | |
| 915 | − void *data) | |
| 935 | +void klp_module_going(struct module *mod) | |
| 916 | 936 | { |
| 917 | − int ret; | |
| 918 | − struct module *mod = data; | |
| 919 | 937 | struct klp_patch *patch; |
| 920 | 938 | struct klp_object *obj; |
| 921 | 939 | |
| 922 | − if (action != MODULE_STATE_COMING && action != MODULE_STATE_GOING) | |
| 923 | − return 0; | |
| 940 | + if (WARN_ON(mod->state != MODULE_STATE_GOING && | |
| 941 | + mod->state != MODULE_STATE_COMING)) | |
| 942 | + return; | |
| 924 | 943 | |
| 925 | 944 | mutex_lock(&klp_mutex); |
| 926 | − | |
| 927 | 945 | /* |
| 928 | − * Each module has to know that the notifier has been called. | |
| 929 | − * We never know what module will get patched by a new patch. | |
| 946 | + * Each module has to know that klp_module_going() | |
| 947 | + * has been called. We never know what module will | |
| 948 | + * get patched by a new patch. | |
| 930 | 949 | */ |
| 931 | − if (action == MODULE_STATE_COMING) | |
| 932 | − mod->klp_alive = true; | |
| 933 | − else /* MODULE_STATE_GOING */ | |
| 934 | − mod->klp_alive = false; | |
| 950 | + mod->klp_alive = false; | |
| 935 | 951 | |
| 936 | 952 | list_for_each_entry(patch, &klp_patches, list) { |
| 937 | 953 | klp_for_each_object(patch, obj) { |
| 938 | 954 | if (!klp_is_module(obj) || strcmp(obj->name, mod->name)) |
| 939 | 955 | continue; |
| 940 | 956 | |
| 941 | − if (action == MODULE_STATE_COMING) { | |
| 942 | − obj->mod = mod; | |
| 943 | − ret = klp_module_notify_coming(patch, obj); | |
| 944 | − if (ret) { | |
| 945 | − obj->mod = NULL; | |
| 946 | − pr_warn("patch '%s' is in an inconsistent state!\n", | |
| 947 | − patch->mod->name); | |
| 948 | − } | |
| 949 | − } else /* MODULE_STATE_GOING */ | |
| 950 | − klp_module_notify_going(patch, obj); | |
| 957 | + if (patch->state != KLP_DISABLED) { | |
| 958 | + pr_notice("reverting patch '%s' on unloading module '%s'\n", | |
| 959 | + patch->mod->name, obj->mod->name); | |
| 960 | + klp_disable_object(obj); | |
| 961 | + } | |
| 951 | 962 | |
| 963 | + klp_free_object_loaded(obj); | |
| 952 | 964 | break; |
| 953 | 965 | } |
| 954 | 966 | } |
| 955 | 967 | |
| 956 | 968 | mutex_unlock(&klp_mutex); |
| 957 | − | |
| 958 | − return 0; | |
| 959 | 969 | } |
| 960 | 970 | |
| 961 | −static struct notifier_block klp_module_nb = { | |
| 962 | − .notifier_call = klp_module_notify, | |
| 963 | − .priority = INT_MIN+1, /* called late but before ftrace notifier */ | |
| 964 | −}; | |
| 965 | − | |
| 966 | 971 | static int __init klp_init(void) |
| 967 | 972 | { |
| 968 | 973 | int ret; |
| @@ -973,21 +978,11 @@ static int __init klp_init(void) · 4 unmodified lines | ||
| 973 | 978 | return -EINVAL; |
| 974 | 979 | } |
| 975 | 980 | |
| 976 | − ret = register_module_notifier(&klp_module_nb); | |
| 977 | − if (ret) | |
| 978 | − return ret; | |
| 979 | − | |
| 980 | 981 | klp_root_kobj = kobject_create_and_add("livepatch", kernel_kobj); |
| 981 | − if (!klp_root_kobj) { | |
| 982 | − ret = -ENOMEM; | |
| 983 | − goto unregister; | |
| 984 | − } | |
| 982 | + if (!klp_root_kobj) | |
| 983 | + return -ENOMEM; | |
| 985 | 984 | |
| 986 | 985 | return 0; |
| 987 | − | |
| 988 | −unregister: | |
| 989 | − unregister_module_notifier(&klp_module_nb); | |
| 990 | − return ret; | |
| 991 | 986 | } |
| 992 | 987 | |
| 993 | 988 | module_init(klp_init); |