Commit 8a141dd7 authored by Alexei Starovoitov's avatar Alexei Starovoitov Committed by Daniel Borkmann
Browse files

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: 0567d680 ("ftrace: Add modify_ftrace_direct()")
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Reviewed-by: default avatarSteven Rostedt (VMware) <rostedt@goodmis.org>
Link: https://lore.kernel.org/bpf/20210316195815.34714-1-alexei.starovoitov@gmail.com
parent 31254dc9

kernel/trace/ftrace.c

+38 −5
Original line number Diff line number Diff line
@@ -5045,6 +5045,20 @@ struct ftrace_direct_func *ftrace_find_direct_func(unsigned long addr) 
	return NULL;

}



static struct ftrace_direct_func *ftrace_alloc_direct_func(unsigned long addr)

{

	struct ftrace_direct_func *direct;



	direct = kmalloc(sizeof(*direct), GFP_KERNEL);

	if (!direct)

		return NULL;

	direct->addr = addr;

	direct->count = 0;

	list_add_rcu(&direct->next, &ftrace_direct_funcs);

	ftrace_direct_func_count++;

	return direct;

}



/**

 * register_ftrace_direct - Call a custom trampoline directly

 * @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) 


	direct = ftrace_find_direct_func(addr);

	if (!direct) {

		direct = kmalloc(sizeof(*direct), GFP_KERNEL);

		direct = ftrace_alloc_direct_func(addr);

		if (!direct) {

			kfree(entry);

			goto out_unlock;

		}

		direct->addr = addr;

		direct->count = 0;

		list_add_rcu(&direct->next, &ftrace_direct_funcs);

		ftrace_direct_func_count++;

	}



	entry->ip = ip;
@@ -5329,6 +5339,7 @@ int __weak ftrace_modify_direct_caller(struct ftrace_func_entry *entry, 
int modify_ftrace_direct(unsigned long ip,

			 unsigned long old_addr, unsigned long new_addr)

{

	struct ftrace_direct_func *direct, *new_direct = NULL;

	struct ftrace_func_entry *entry;

	struct dyn_ftrace *rec;

	int ret = -ENODEV;
@@ -5344,6 +5355,20 @@ int modify_ftrace_direct(unsigned long ip, 
	if (entry->direct != old_addr)

		goto out_unlock;



	direct = ftrace_find_direct_func(old_addr);

	if (WARN_ON(!direct))

		goto out_unlock;

	if (direct->count > 1) {

		ret = -ENOMEM;

		new_direct = ftrace_alloc_direct_func(new_addr);

		if (!new_direct)

			goto out_unlock;

		direct->count--;

		new_direct->count++;

	} else {

		direct->addr = new_addr;

	}



	/*

	 * If there's no other ftrace callback on the rec->ip location,

	 * then it can be changed directly by the architecture.
@@ -5357,6 +5382,14 @@ int modify_ftrace_direct(unsigned long ip, 
		ret = 0;

	}



	if (unlikely(ret && new_direct)) {

		direct->count++;

		list_del_rcu(&new_direct->next);

		synchronize_rcu_tasks();

		kfree(new_direct);

		ftrace_direct_func_count--;

	}



 out_unlock:

	mutex_unlock(&ftrace_lock);

	mutex_unlock(&direct_mutex);