klp_check_stack_func() decides which address range to look for on a
task's stack by asking whether the func preceding @func on
ops->func_stack is the original kernel function or another livepatch's
replacement. It uses list_is_singular(&ops->func_stack), which only
tells "one func on this stack" from "more than one". That assumes every
klp_func of a patch gets its own func_stack.
Aliases break the assumption. Several symbols can share one address:
ffffffff8ed7fef0 t __do_sys_fork
ffffffff8ed7fef0 T __ia32_sys_fork
ffffffff8ed7fef0 T __x64_sys_fork
klp_find_ops() looks the ops up by func->old_func, i.e. by address, so
two klp_funcs of the same patch naming two of these symbols resolve to
the same klp_ops and are both pushed onto one func_stack.
The stack is then head -> B -> A. A is the last node and does
correspond to the original function, but list_is_singular() is false, so
the "previously patched function" branch runs: list_next_entry() applies
container_of() to &ops->func_stack, treating the list head as a struct
klp_func, and reads func_addr/func_size from past the object. Besides
the out-of-bounds read, the bogus range can keep matching stack entries,
so tasks that are safe to switch get -EAGAIN forever and the transition
never completes.
Test whether @func itself is the last entry instead. The answer is
derived from @func's position rather than from the list length, so it
holds however many klp_funcs share a func_stack and never steps onto the
list head. A single-entry stack is still trivially last, so existing
behaviour is unchanged.
Fixes: d83a7cb375ee ("livepatch: change to a per-task consistency model")
Signed-off-by: Harry Hsu <[email protected]>
---
kernel/livepatch/transition.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index 2351a19ac2a9..8d6e3a58101f 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -223,7 +223,7 @@ static int klp_check_stack_func(struct klp_func *func,
unsigned long *entries,
*/
ops = klp_find_ops(func->old_func);
- if (list_is_singular(&ops->func_stack)) {
+ if (list_is_last(&func->stack_node, &ops->func_stack)) {
/* original function */
func_addr = (unsigned long)func->old_func;
func_size = func->old_size;
--
2.43.0
