kobject_del() is called from kobject_put(), and after the klp_patch kobject is deleted, any show()/store() are done. Once the klp_patch object is removed from list and prepared for releasing, no need to hold the global mutex of klp_mutex, so move the freeing outside of klp_mutex. Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx> --- kernel/livepatch/core.c | 30 ++++++++++++++++++------------ kernel/livepatch/core.h | 3 +-- kernel/livepatch/transition.c | 23 +++++++++++++++++------ kernel/livepatch/transition.h | 2 +- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index b967b4b0071b..9ede093d699a 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -327,7 +327,8 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs, * /sys/kernel/livepatch/<patch>/<object> * /sys/kernel/livepatch/<patch>/<object>/<function,sympos> */ -static int __klp_disable_patch(struct klp_patch *patch); +static int __klp_disable_patch(struct klp_patch *patch, + struct list_head *to_free); static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf, size_t count) @@ -335,6 +336,7 @@ static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr, struct klp_patch *patch; int ret; bool enabled; + LIST_HEAD(to_free); ret = kstrtobool(buf, &enabled); if (ret) @@ -360,13 +362,15 @@ static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr, if (patch == klp_transition_patch) klp_reverse_transition(); else if (!enabled) - ret = __klp_disable_patch(patch); + ret = __klp_disable_patch(patch, &to_free); else ret = -EINVAL; out: mutex_unlock(&klp_mutex); + klp_free_patches_async(&to_free); + if (ret) return ret; return count; @@ -693,20 +697,19 @@ static void klp_free_patch_work_fn(struct work_struct *work) klp_free_patch_finish(patch); } -void klp_free_patch_async(struct klp_patch *patch) +static void klp_free_patch_async(struct klp_patch *patch) { klp_free_patch_start(patch); schedule_work(&patch->free_work); } -void klp_free_replaced_patches_async(struct klp_patch *new_patch) +void klp_free_patches_async(struct list_head *to_free) { - struct klp_patch *old_patch, *tmp_patch; + struct klp_patch *patch, *tmp_patch; - klp_for_each_patch_safe(old_patch, tmp_patch) { - if (old_patch == new_patch) - return; - klp_free_patch_async(old_patch); + list_for_each_entry_safe(patch, tmp_patch, to_free, list) { + list_del_init(&patch->list); + klp_free_patch_async(patch); } } @@ -915,7 +918,8 @@ static int klp_init_patch(struct klp_patch *patch) return 0; } -static int __klp_disable_patch(struct klp_patch *patch) +static int __klp_disable_patch(struct klp_patch *patch, + struct list_head *to_free) { struct klp_object *obj; @@ -942,7 +946,7 @@ static int __klp_disable_patch(struct klp_patch *patch) klp_start_transition(); patch->enabled = false; - klp_try_complete_transition(); + klp_try_complete_transition(to_free); return 0; } @@ -951,6 +955,7 @@ static int __klp_enable_patch(struct klp_patch *patch) { struct klp_object *obj; int ret; + LIST_HEAD(unused); if (klp_transition_patch) return -EBUSY; @@ -992,7 +997,8 @@ static int __klp_enable_patch(struct klp_patch *patch) klp_start_transition(); patch->enabled = true; - klp_try_complete_transition(); + klp_try_complete_transition(&unused); + WARN_ON_ONCE(!list_empty(&unused)); return 0; err: diff --git a/kernel/livepatch/core.h b/kernel/livepatch/core.h index 38209c7361b6..8ff97745ba40 100644 --- a/kernel/livepatch/core.h +++ b/kernel/livepatch/core.h @@ -13,8 +13,7 @@ extern struct list_head klp_patches; #define klp_for_each_patch(patch) \ list_for_each_entry(patch, &klp_patches, list) -void klp_free_patch_async(struct klp_patch *patch); -void klp_free_replaced_patches_async(struct klp_patch *new_patch); +void klp_free_patches_async(struct list_head *to_free); void klp_unpatch_replaced_patches(struct klp_patch *new_patch); void klp_discard_nops(struct klp_patch *new_patch); diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c index 291b857a6e20..a9ebc9c5db02 100644 --- a/kernel/livepatch/transition.c +++ b/kernel/livepatch/transition.c @@ -32,12 +32,16 @@ static unsigned int klp_signals_cnt; */ static void klp_transition_work_fn(struct work_struct *work) { + LIST_HEAD(to_free); + mutex_lock(&klp_mutex); if (klp_transition_patch) - klp_try_complete_transition(); + klp_try_complete_transition(&to_free); mutex_unlock(&klp_mutex); + + klp_free_patches_async(&to_free); } static DECLARE_DELAYED_WORK(klp_transition_work, klp_transition_work_fn); @@ -384,7 +388,7 @@ static void klp_send_signals(void) * * If any tasks are still stuck in the initial patch state, schedule a retry. */ -void klp_try_complete_transition(void) +void klp_try_complete_transition(struct list_head *to_free) { unsigned int cpu; struct task_struct *g, *task; @@ -449,10 +453,17 @@ void klp_try_complete_transition(void) * klp_complete_transition() but it is called also * from klp_cancel_transition(). */ - if (!patch->enabled) - klp_free_patch_async(patch); - else if (patch->replace) - klp_free_replaced_patches_async(patch); + if (!patch->enabled) { + list_move(&patch->list, to_free); + } else if (patch->replace) { + struct klp_patch *old_patch, *tmp_patch; + + klp_for_each_patch_safe(old_patch, tmp_patch) { + if (old_patch == patch) + break; + list_move(&old_patch->list, to_free); + } + } } /* diff --git a/kernel/livepatch/transition.h b/kernel/livepatch/transition.h index 322db16233de..20e3a5a0cbce 100644 --- a/kernel/livepatch/transition.h +++ b/kernel/livepatch/transition.h @@ -9,7 +9,7 @@ extern struct klp_patch *klp_transition_patch; void klp_init_transition(struct klp_patch *patch, int state); void klp_cancel_transition(void); void klp_start_transition(void); -void klp_try_complete_transition(void); +void klp_try_complete_transition(struct list_head *to_free); void klp_reverse_transition(void); void klp_force_transition(void); -- 2.31.1