From 747f7a2901174c9afa805dddfb7b24db6f65e985 Mon Sep 17 00:00:00 2001 From: Rik van Riel Date: Mon, 8 Aug 2022 15:00:19 -0400 Subject: livepatch: fix race between fork and KLP transition The KLP transition code depends on the TIF_PATCH_PENDING and the task->patch_state to stay in sync. On a normal (forward) transition, TIF_PATCH_PENDING will be set on every task in the system, while on a reverse transition (after a failed forward one) first TIF_PATCH_PENDING will be cleared from every task, followed by it being set on tasks that need to be transitioned back to the original code. However, the fork code copies over the TIF_PATCH_PENDING flag from the parent to the child early on, in dup_task_struct and setup_thread_stack. Much later, klp_copy_process will set child->patch_state to match that of the parent. However, the parent's patch_state may have been changed by KLP loading or unloading since it was initially copied over into the child. This results in the KLP code occasionally hitting this warning in klp_complete_transition: for_each_process_thread(g, task) { WARN_ON_ONCE(test_tsk_thread_flag(task, TIF_PATCH_PENDING)); task->patch_state = KLP_UNDEFINED; } Set, or clear, the TIF_PATCH_PENDING flag in the child task depending on whether or not it is needed at the time klp_copy_process is called, at a point in copy_process where the tasklist_lock is held exclusively, preventing races with the KLP code. The KLP code does have a few places where the state is changed without the tasklist_lock held, but those should not cause problems because klp_update_patch_state(current) cannot be called while the current task is in the middle of fork, klp_check_and_switch_task() which is called under the pi_lock, which prevents rescheduling, and manipulation of the patch state of idle tasks, which do not fork. This should prevent this warning from triggering again in the future, and close the race for both normal and reverse transitions. Signed-off-by: Rik van Riel Reported-by: Breno Leitao Reviewed-by: Petr Mladek Acked-by: Josh Poimboeuf Fixes: d83a7cb375ee ("livepatch: change to a per-task consistency model") Cc: stable@kernel.org Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220808150019.03d6a67b@imladris.surriel.com --- kernel/livepatch/transition.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) (limited to 'kernel/livepatch') diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c index 5d03a2ad1066..30187b1d8275 100644 --- a/kernel/livepatch/transition.c +++ b/kernel/livepatch/transition.c @@ -610,9 +610,23 @@ void klp_reverse_transition(void) /* Called from copy_process() during fork */ void klp_copy_process(struct task_struct *child) { - child->patch_state = current->patch_state; - /* TIF_PATCH_PENDING gets copied in setup_thread_stack() */ + /* + * The parent process may have gone through a KLP transition since + * the thread flag was copied in setup_thread_stack earlier. Bring + * the task flag up to date with the parent here. + * + * The operation is serialized against all klp_*_transition() + * operations by the tasklist_lock. The only exception is + * klp_update_patch_state(current), but we cannot race with + * that because we are current. + */ + if (test_tsk_thread_flag(current, TIF_PATCH_PENDING)) + set_tsk_thread_flag(child, TIF_PATCH_PENDING); + else + clear_tsk_thread_flag(child, TIF_PATCH_PENDING); + + child->patch_state = current->patch_state; } /* -- cgit v1.2.3-59-g8ed1b From 66d8529d0f0423bc0fc249a5620c342c122981fb Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Tue, 30 Aug 2022 19:28:55 +0800 Subject: livepatch: Add a missing newline character in klp_module_coming() The error message is not printed immediately because it does not end with a newline character. Before: root@localhost:~# insmod vmlinux.ko insmod: ERROR: could not insert module vmlinux.ko: Invalid parameters After: root@localhost:~# insmod vmlinux.ko [ 43.982558] livepatch: vmlinux.ko: invalid module name insmod: ERROR: could not insert module vmlinux.ko: Invalid parameters Fixes: dcf550e52f56 ("livepatch: Disallow vmlinux.ko") Signed-off-by: Zhen Lei Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220830112855.749-1-thunder.leizhen@huawei.com --- kernel/livepatch/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'kernel/livepatch') diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index bc475e62279d..42f7e716d56b 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -1171,7 +1171,7 @@ int klp_module_coming(struct module *mod) return -EINVAL; if (!strcmp(mod->name, "vmlinux")) { - pr_err("vmlinux.ko: invalid module name"); + pr_err("vmlinux.ko: invalid module name\n"); return -EINVAL; } -- cgit v1.2.3-59-g8ed1b From bb26cfd9e77e8dadd4be2ca154017bde9326cd4b Mon Sep 17 00:00:00 2001 From: Song Liu Date: Fri, 2 Sep 2022 13:52:07 -0700 Subject: livepatch: add sysfs entry "patched" for each klp_object Add per klp_object sysfs entry "patched". It makes it easier to debug typos in the module name. Signed-off-by: Song Liu Reviewed-by: Joe Lawrence [pmladek@suse.com: Updated kernel version when the sysfs file will be introduced] Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220902205208.3117798-2-song@kernel.org --- Documentation/ABI/testing/sysfs-kernel-livepatch | 8 ++++++++ kernel/livepatch/core.c | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+) (limited to 'kernel/livepatch') diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch b/Documentation/ABI/testing/sysfs-kernel-livepatch index bea7bd5a1d5f..a5df9b4910dc 100644 --- a/Documentation/ABI/testing/sysfs-kernel-livepatch +++ b/Documentation/ABI/testing/sysfs-kernel-livepatch @@ -55,6 +55,14 @@ Description: The object directory contains subdirectories for each function that is patched within the object. +What: /sys/kernel/livepatch///patched +Date: August 2022 +KernelVersion: 6.1.0 +Contact: live-patching@vger.kernel.org +Description: + An attribute which indicates whether the object is currently + patched. + What: /sys/kernel/livepatch/// Date: Nov 2014 KernelVersion: 3.19.0 diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index bc475e62279d..67eb9f9168f3 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -325,6 +325,7 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs, * /sys/kernel/livepatch//transition * /sys/kernel/livepatch//force * /sys/kernel/livepatch// + * /sys/kernel/livepatch///patched * /sys/kernel/livepatch/// */ static int __klp_disable_patch(struct klp_patch *patch); @@ -431,6 +432,22 @@ static struct attribute *klp_patch_attrs[] = { }; ATTRIBUTE_GROUPS(klp_patch); +static ssize_t patched_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) +{ + struct klp_object *obj; + + obj = container_of(kobj, struct klp_object, kobj); + return sysfs_emit(buf, "%d\n", obj->patched); +} + +static struct kobj_attribute patched_kobj_attr = __ATTR_RO(patched); +static struct attribute *klp_object_attrs[] = { + &patched_kobj_attr.attr, + NULL, +}; +ATTRIBUTE_GROUPS(klp_object); + static void klp_free_object_dynamic(struct klp_object *obj) { kfree(obj->name); @@ -576,6 +593,7 @@ static void klp_kobj_release_object(struct kobject *kobj) static struct kobj_type klp_ktype_object = { .release = klp_kobj_release_object, .sysfs_ops = &kobj_sysfs_ops, + .default_groups = klp_object_groups, }; static void klp_kobj_release_func(struct kobject *kobj) -- cgit v1.2.3-59-g8ed1b