[PATCH stable] xdp: Reset bpf_redirect_info before running a xdp's BPF prog.

[PATCH stable] xdp: Reset bpf_redirect_info before running a xdp's BPF prog.

[Sebastian Andrzej Siewior]

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.

Greg, feel free to decide if this is worth a CVE.

 include/net/xdp.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

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))
-- 
2.49.0

Re: [PATCH stable] xdp: Reset bpf_redirect_info before running a xdp's BPF prog.

[Greg KH]

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

Re: [PATCH stable] xdp: Reset bpf_redirect_info before running a xdp's BPF prog.

[Sebastian Andrzej Siewior]

> > 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

Re: [PATCH stable] xdp: Reset bpf_redirect_info before running a xdp's BPF prog.

[Sebastian Andrzej Siewior]

On 2025-03-17 15:08:52 [+0100], To Greg KH wrote:
> > > 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.

This wasn't picked up yet. Is it the merge window or something wrong
with the submission?
 
Sebastian

[PATCH stable] xdp: Reset bpf_redirect_info before running a xdp's BPF prog.

[Sebastian Andrzej Siewior]

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 but it
seems to work similar.
Affected kernels would be from v4.14 to v6.10.

 include/net/xdp.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

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))
-- 
2.49.0

Re: [PATCH stable] xdp: Reset bpf_redirect_info before running a xdp's BPF prog.

[Greg KH]

On Mon, Apr 14, 2025 at 06:21:20PM +0200, 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 but it
> seems to work similar.
> Affected kernels would be from v4.14 to v6.10.

Does not apply to any tree other than 6.6.y :(