Re: [PATCH v4 0/5] livepatch: Fail object initialization on duplicate patched function

[Harry Hsu <x90613@gmail.com>]

On Fri 2026-10-23, Petr Mladek wrote:
> I wonder how to move forward with this patchset. There seems to be
> three unresolved problems:
>
>   1. How to handle aliased functions when replacing two livepatches
>      with aliased symbols, aka two NOPs on the same stack [1].
>
>      My take: I would fix this later on top of scoped atomic replace.
>               The aliased symbols should conflict.
>   2. Fix the low-level __write_relocate_add() to clean its own
>      mess on failure.
>
>      My take: I would fix this a follow up patch.
>
>   3. Better describe the purpose of klp_write_object_relocs_limited().
>      Or clean its own mess a better way (code duplication)?
>
>      My take:  This requires v5.

For 1, I have a proposal to create only one NOP in the first place,
as in your solution 1 from the earlier mail.


Recap: The problem
-----------

Take two livepatches that are already loaded:
  A patches __do_sys_fork
  B patches __x64_sys_fork        (same address as __do_sys_fork)

and an atomic replace livepatch C that patches neither of them.
klp_add_nops() finds the functions by name and sympos only, so C
inherits two NOPs. Both resolve to the same address and end up on
the same ops->func_stack: [A, B, nop1, nop2]. When the transition
is reverted, the stack goes from nop2 to nop1 instead of back to B.


Proposal
--------

klp_add_nops() runs from klp_init_patch() before klp_init_object(),
so the new NOPs have no address yet. But func->old_func of the old
livepatches is already resolved when their object is loaded. So the
address can be used as a hint when the NOPs are created:

  1. Copy the already resolved address of the old function into the
     new NOP (func->old_func) as an address hint.
  2. Do not create another NOP when a NOP with the same address
     already exists in the object.

The hint is overwritten by klp_find_object_symbol() in
klp_init_object_loaded() with the same value. It is cleared again by
klp_free_object_loaded(), so it never outlives the loaded object.

A sketch of the change in kernel/livepatch/core.c:

+/*
+ * Return true when @obj already has a nop whose address hint is @addr.
+ * The hint is only set when the livepatched object was already loaded.
+ */
+static bool klp_nop_exists_at(struct klp_object *obj, void *addr)
+{
+	struct klp_func *func;
+
+	if (!addr)
+		return false;
+
+	klp_for_each_func(obj, func) {
+		if (func->nop && func->old_func == addr)
+			return true;
+	}
+
+	return false;
+}
+
 static int klp_add_object_nops(struct klp_patch *patch,
 			       struct klp_object *old_obj)
 {
@@
 	klp_for_each_func(old_obj, old_func) {
 		func = klp_find_func(obj, old_func);
 		if (func)
 			continue;

+		/*
+		 * Aliased symbols resolve to the same address. A single nop
+		 * is enough. More nops on the same ops->func_stack would
+		 * break reverting the transition.
+		 */
+		if (klp_nop_exists_at(obj, old_func->old_func))
+			continue;
+
 		func = klp_alloc_func_nop(old_func, obj);
 		if (!func)
 			return -ENOMEM;
+
+		/* Address hint for klp_nop_exists_at(). */
+		func->old_func = old_func->old_func;
 	}

That's only one way I can come up with :D

Best Regards,
Harry