Message ID | 20250317133813.OwHVKUKe@linutronix.de (mailing list archive) |
---|---|
State | Not Applicable |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [stable] xdp: Reset bpf_redirect_info before running a xdp's BPF prog. | expand |
On Mon, Mar 17, 2025 at 02:38:13PM +0100, Sebastian Andrzej Siewior wrote: > Ricardo reported a KASAN discovered use after free in v6.6-stable. > > The syzbot starts a BPF program via xdp_test_run_batch() which assigns > ri->tgt_value via dev_hash_map_redirect() and the return code isn't > XDP_REDIRECT it looks like nonsense. So the output in > bpf_warn_invalid_xdp_action() appears once. > Then the TUN driver runs another BPF program (on the same CPU) which > returns XDP_REDIRECT without setting ri->tgt_value first. It invokes > bpf_trace_printk() to print four characters and obtain the required > return value. This is enough to get xdp_do_redirect() invoked which > then accesses the pointer in tgt_value which might have been already > deallocated. > > This problem does not affect upstream because since commit > 401cb7dae8130 ("net: Reference bpf_redirect_info via task_struct on PREEMPT_RT.") > > the per-CPU variable is referenced via task's task_struct and exists on > the stack during NAPI callback. Therefore it is cleared once before the > first invocation and remains valid within the RCU section of the NAPI > callback. > > Instead of performing the huge backport of the commit (plus its fix ups) > here is an alternative version which only resets the variable in > question prior invoking the BPF program. > > Acked-by: Toke Høiland-Jørgensen <toke@kernel.org> > Reported-by: Ricardo Cañuelo Navarro <rcn@igalia.com> > Closes: https://lore.kernel.org/all/20250226-20250204-kasan-slab-use-after-free-read-in-dev_map_enqueue__submit-v3-0-360efec441ba@igalia.com/ > Fixes: 97f91a7cf04ff ("bpf: add bpf_redirect_map helper routine") > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > > I discussed this with Toke, thread starts at > https://lore.kernel.org/all/20250313183911.SPAmGLyw@linutronix.de/ > > The commit, which this by accident, is part of v6.11-rc1. > I added the commit introducing map redirects as the origin of the > problem which is v4.14-rc1. The code is a bit different there it seems > to work similar. What stable tree(s) is this for? Just 6.6.y? Why not older ones? > Greg, feel free to decide if this is worth a CVE. That's not how CVEs are assigned :) If you want one, please read the in-tree documentation we have for that. thanks, greg k-h
> > I added the commit introducing map redirects as the origin of the > > problem which is v4.14-rc1. The code is a bit different there it seems > > to work similar. > > What stable tree(s) is this for? Just 6.6.y? Why not older ones? I didn't say just v6.6.y. The commit introducing the problem is in v4.14-rc1 so I would say all the way down for the supported trees. Just let me know if it does not apply for some of the older kernel. > > Greg, feel free to decide if this is worth a CVE. > > That's not how CVEs are assigned :) > > If you want one, please read the in-tree documentation we have for that. I don't need one but it is tempting to go through the new process :). If it does not make your handling here easier (since you have two different patches for one issue) there is no need for it from my side. Thank you. > thanks, > > greg k-h Sebastian
diff --git a/include/net/xdp.h b/include/net/xdp.h index de08c8e0d1348..b39ac83618a55 100644 --- a/include/net/xdp.h +++ b/include/net/xdp.h @@ -486,7 +486,14 @@ static __always_inline u32 bpf_prog_run_xdp(const struct bpf_prog *prog, * under local_bh_disable(), which provides the needed RCU protection * for accessing map entries. */ - u32 act = __bpf_prog_run(prog, xdp, BPF_DISPATCHER_FUNC(xdp)); + struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info); + u32 act; + + if (ri->map_id || ri->map_type) { + ri->map_id = 0; + ri->map_type = BPF_MAP_TYPE_UNSPEC; + } + act = __bpf_prog_run(prog, xdp, BPF_DISPATCHER_FUNC(xdp)); if (static_branch_unlikely(&bpf_master_redirect_enabled_key)) { if (act == XDP_TX && netif_is_bond_slave(xdp->rxq->dev))