Thanks for catching this.
I've updated the duplicate-address check in klp_init_object_loaded() to
only reject the pair when both klp_funcs are nops:
if (prev_func->old_func == func->old_func) {
if (prev_func->nop && func->nop)
continue;
pr_err(...);
return -EINVAL;
}
nops added by klp_add_nops() for two replaced aliases are
interchangeable. Both just fall through to the original function, so
allowing that combination fixes exactly the scenario you described: an
atomic replace patch that inherits nops for __do_sys_fork and
__x64_sys_fork from two separate previous patches will now load fine.
The check still rejects two non-nop funcs that resolve to the same
address, since that's still genuinely ambiguous. It also still rejects
a non-nop func colliding with an auto-generated nop for its alias.
klp_add_nops() always appends nops after the explicitly-listed funcs,
and klp_patch_func() always pushes new entries onto the head of
ops->func_stack, so allowing that combination would let the nop
silently land on top and disable the real replacement instead of
failing loudly.
Petr, do you have any further changes on your side for this series?
And once this fix gets an Ack, would you like me to send v5, or would
it be more convenient for you to fold my commit in and send it
together with yours? Happy to go either way, just let me know what
works best for you.
Harry
