From: Feng Yang <[email protected]>

BPF fexit programs run after the traced function returns, while their
context still contains the original function argument values. A traced
function is free to consume an skb argument before returning, so the
pointer seen by fexit can already be stale.

The verifier checks that the first argument to bpf_skb_output() has the
BTF type of struct sk_buff, but that does not establish its lifetime.
bpf_skb_event_output() then dereferences skb->len and can trigger a
use-after-free.

Do not expose bpf_skb_output() to tracing programs which can run after
the target: fexit, fexit.multi, fsession and fsession.multi. Keep it
available to fentry and other tracing attach types where it is already
supported. fsession must be rejected because the same program runs on
both entry and return and the verifier cannot prove that a helper call
is entry-only.

Fixes: fec56f5890d9 ("bpf: Introduce BPF trampoline")
Reported-by: Quan Sun <[email protected]>
Reported-by: Yinhao Hu <[email protected]>
Reported-by: Kaiyan Mei <[email protected]>
Closes: 
https://lore.kernel.org/all/[email protected]/
Signed-off-by: Yun Lu <[email protected]>
Signed-off-by: Feng Yang <[email protected]>
---
v2: Add inline.

v1: https://lore.kernel.org/all/[email protected]/
---
 kernel/trace/bpf_trace.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 29260951aa87..29c83e2938cd 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1339,6 +1339,20 @@ static inline bool is_trace_fsession(const struct 
bpf_prog *prog)
                prog->expected_attach_type == BPF_TRACE_FSESSION_MULTI);
 }
 
+static inline bool tracing_prog_may_run_after_target(const struct bpf_prog 
*prog)
+{
+       /* The target may consume pointer arguments before these programs run. 
*/
+       switch (prog->expected_attach_type) {
+       case BPF_TRACE_FEXIT:
+       case BPF_TRACE_FEXIT_MULTI:
+       case BPF_TRACE_FSESSION:
+       case BPF_TRACE_FSESSION_MULTI:
+               return true;
+       default:
+               return false;
+       }
+}
+
 static const struct bpf_func_proto *
 kprobe_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -1730,6 +1744,8 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const 
struct bpf_prog *prog)
        switch (func_id) {
 #ifdef CONFIG_NET
        case BPF_FUNC_skb_output:
+               if (tracing_prog_may_run_after_target(prog))
+                       return NULL;
                return &bpf_skb_output_proto;
        case BPF_FUNC_xdp_output:
                return &bpf_xdp_output_proto;
-- 
2.27.0


Reply via email to