Re: [RFC] Use after free in BPF/ XDP during XDP_REDIRECT

["Toke Høiland-Jørgensen" <toke@kernel.org>]

Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes:

> On 2025-03-14 10:21:15 [+0100], Toke Høiland-Jørgensen wrote:
>> Hmm, how about putting the reset (essentially the changes you have
>> above) into bpf_prog_run_xdp() itself, before executing the BPF program?
>
> That would be the snippet below. It does work as far as the testcase
> goes. It is just and unconditional write which might look like a waste
> but given the circumstances…

Hmm, yeah, it would slow down applications that never redirect, I
suppose. Hmm, we could avoid the write by checking the values first? See
below.

> While at it, is there anything that ensures that only bpf_prog_run_xdp()
> can invoke the map_redirect callback? Mainline only assigns the task
> pointer in NAPI callback so any usage outside of bpf_prog_run_xdp() will
> lead to a segfault and I haven't seen a report yet so…

Yes, the verifier restricts which program types can call the
map_redirect helper.


> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -486,7 +486,12 @@ 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;
> +

Add an if here like

if (ri->map_id | ri->map_type) { /* single | to make it a single branch */

> +	ri->map_id = INT_MAX;
> +	ri->map_type = BPF_MAP_TYPE_UNSPEC;

}

Also, ri->map_id should be set to 0, not INT_MAX.

-Toke