On Thu 2018-06-07 11:29:48, Miroslav Benes wrote: > We decided to deny the patched modules to be removed. If it proves to be > a major drawback for users, we can still implement a different approach. > > The reference of a patched module has to be taken regardless of a > patch's state. Thus it is not taken and dropped in enable/disable paths, > but in register/unregister paths. > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -739,6 +746,14 @@ static int klp_init_object_loaded(struct klp_patch *patch, > } > } > > + /* > + * Do not allow patched modules to be removed. > + * > + * All callers of klp_init_object_loaded() set obj->mod. > + */ > + if (klp_is_module(obj) && !try_module_get(obj->mod)) > + return -ENODEV; > + > return 0; > } This looks strange. We try to take the reference after we do all actions needed for a loaded module. There is nothing that would prevent obj->mod to get into the going state. Note that it was safe (except for the relocated symbols) because the module was not able to really go until it called klp_module_going(). But you removed this last break by the 3rd patch. I suggest another approach: I would rename klp_find_object_module() to klp_get_object_module() and get the reference there. Note that it should be fine to call it later in klp_init_object() (before klp_init_object_loaded()). This would solve klp_init_patch(). We will also need to get the reference in klp_module_coming(). It can be called anywhere there. If it fails, we will block loading the module. Note that the mod->klp_alive logic will still be necessary. It solves various races when both the patch and the patched module are loaded in parallel. Sigh, this also means that we still could call klp_init_object_loaded() and apply reloacations even when the patched module fails to loaded later from other reasons. This means that this approach does not solve the original problem completely. I wonder how complicated would be the arch-specific part that would clean up relocations when the module is unloaded. Best Regards, Petr -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
