Skip to content

Commit 8a141dd

Browse files
Alexei Starovoitovborkmann
authored andcommitted
ftrace: Fix modify_ftrace_direct.
The following sequence of commands: register_ftrace_direct(ip, addr1); modify_ftrace_direct(ip, addr1, addr2); unregister_ftrace_direct(ip, addr2); will cause the kernel to warn: [ 30.179191] WARNING: CPU: 2 PID: 1961 at kernel/trace/ftrace.c:5223 unregister_ftrace_direct+0x130/0x150 [ 30.180556] CPU: 2 PID: 1961 Comm: test_progs W O 5.12.0-rc2-00378-g86bc10a0a711-dirty #3246 [ 30.182453] RIP: 0010:unregister_ftrace_direct+0x130/0x150 When modify_ftrace_direct() changes the addr from old to new it should update the addr stored in ftrace_direct_funcs. Otherwise the final unregister_ftrace_direct() won't find the address and will cause the splat. Fixes: 0567d68 ("ftrace: Add modify_ftrace_direct()") Signed-off-by: Alexei Starovoitov <[email protected]> Signed-off-by: Daniel Borkmann <[email protected]> Reviewed-by: Steven Rostedt (VMware) <[email protected]> Link: https://lore.kernel.org/bpf/[email protected]
1 parent 31254dc commit 8a141dd

File tree

1 file changed

+38
-5
lines changed

1 file changed

+38
-5
lines changed

kernel/trace/ftrace.c

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5045,6 +5045,20 @@ struct ftrace_direct_func *ftrace_find_direct_func(unsigned long addr)
50455045
return NULL;
50465046
}
50475047

5048+
static struct ftrace_direct_func *ftrace_alloc_direct_func(unsigned long addr)
5049+
{
5050+
struct ftrace_direct_func *direct;
5051+
5052+
direct = kmalloc(sizeof(*direct), GFP_KERNEL);
5053+
if (!direct)
5054+
return NULL;
5055+
direct->addr = addr;
5056+
direct->count = 0;
5057+
list_add_rcu(&direct->next, &ftrace_direct_funcs);
5058+
ftrace_direct_func_count++;
5059+
return direct;
5060+
}
5061+
50485062
/**
50495063
* register_ftrace_direct - Call a custom trampoline directly
50505064
* @ip: The address of the nop at the beginning of a function
@@ -5120,15 +5134,11 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
51205134

51215135
direct = ftrace_find_direct_func(addr);
51225136
if (!direct) {
5123-
direct = kmalloc(sizeof(*direct), GFP_KERNEL);
5137+
direct = ftrace_alloc_direct_func(addr);
51245138
if (!direct) {
51255139
kfree(entry);
51265140
goto out_unlock;
51275141
}
5128-
direct->addr = addr;
5129-
direct->count = 0;
5130-
list_add_rcu(&direct->next, &ftrace_direct_funcs);
5131-
ftrace_direct_func_count++;
51325142
}
51335143

51345144
entry->ip = ip;
@@ -5329,6 +5339,7 @@ int __weak ftrace_modify_direct_caller(struct ftrace_func_entry *entry,
53295339
int modify_ftrace_direct(unsigned long ip,
53305340
unsigned long old_addr, unsigned long new_addr)
53315341
{
5342+
struct ftrace_direct_func *direct, *new_direct = NULL;
53325343
struct ftrace_func_entry *entry;
53335344
struct dyn_ftrace *rec;
53345345
int ret = -ENODEV;
@@ -5344,6 +5355,20 @@ int modify_ftrace_direct(unsigned long ip,
53445355
if (entry->direct != old_addr)
53455356
goto out_unlock;
53465357

5358+
direct = ftrace_find_direct_func(old_addr);
5359+
if (WARN_ON(!direct))
5360+
goto out_unlock;
5361+
if (direct->count > 1) {
5362+
ret = -ENOMEM;
5363+
new_direct = ftrace_alloc_direct_func(new_addr);
5364+
if (!new_direct)
5365+
goto out_unlock;
5366+
direct->count--;
5367+
new_direct->count++;
5368+
} else {
5369+
direct->addr = new_addr;
5370+
}
5371+
53475372
/*
53485373
* If there's no other ftrace callback on the rec->ip location,
53495374
* then it can be changed directly by the architecture.
@@ -5357,6 +5382,14 @@ int modify_ftrace_direct(unsigned long ip,
53575382
ret = 0;
53585383
}
53595384

5385+
if (unlikely(ret && new_direct)) {
5386+
direct->count++;
5387+
list_del_rcu(&new_direct->next);
5388+
synchronize_rcu_tasks();
5389+
kfree(new_direct);
5390+
ftrace_direct_func_count--;
5391+
}
5392+
53605393
out_unlock:
53615394
mutex_unlock(&ftrace_lock);
53625395
mutex_unlock(&direct_mutex);

0 commit comments

Comments
 (0)