Thread (6 messages) 6 messages, 4 authors, 2022-07-29

Re: [PATCH bpf-next] bpf: Fix NULL pointer dereference when registering bpf trampoline

From: Xu Kuohai <xukuohai@huaweicloud.com>
Date: 2022-07-28 12:56:36
Also in: bpf

On 7/28/2022 8:27 PM, Jiri Olsa wrote:
On Thu, Jul 28, 2022 at 07:40:48AM -0400, Xu Kuohai wrote:
quoted
From: Xu Kuohai <redacted>
SNIP
quoted
It's caused by a NULL tr->fops passed to ftrace_set_filter_ip(). tr->fops
is initialized to NULL and is assigned to an allocated memory address if
CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS is enabled. Since there is no
direct call on arm64 yet, the config can't be enabled.

To fix it, call ftrace_set_filter_ip() only if tr->fops is not NULL.

Fixes: 00963a2e75a8 ("bpf: Support bpf_trampoline on functions with IPMODIFY (e.g. livepatch)")
Reported-by: Bruno Goncalves <redacted>
Signed-off-by: Xu Kuohai <redacted>
Tested-by: Bruno Goncalves <redacted>
Acked-by: Song Liu <redacted>
---
  kernel/bpf/trampoline.c | 11 +++++++++--
  1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 42e387a12694..0d5a9e0b9a7b 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -255,8 +255,15 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
  		return -ENOENT;
  
  	if (tr->func.ftrace_managed) {
-		ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 0);
-		ret = register_ftrace_direct_multi(tr->fops, (long)new_addr);
+		if (tr->fops)
+			ret = ftrace_set_filter_ip(tr->fops, (unsigned long)ip,
+						   0, 0);
+		else
+			ret = -ENOTSUPP;
+
+		if (!ret)
+			ret = register_ftrace_direct_multi(tr->fops,
+							   (long)new_addr);
  	} else {
  		ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, NULL, new_addr);
  	}
do we need to do the same also in unregister_fentry and modify_fentry ?
No need for now, this is the only place where we call ftrace_set_filter_ip().

tr->fops is passed to ftrace_set_filter_ip() and *ftrace_direct_multi()
functions, and when CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS is not enabled,
the *ftrace_direct_multi()s do nothing except returning an error code, so
it's safe to pass NULL to them.
jirka
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help