Re: [PATCH net v2 2/3] net/sched: Handle TC_ACT_REDIRECT from qdisc filter chains

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Uninitialized kernel stack memory in `struct bpf_net_context` can be leaked via eBPF program redirect.
--

commit 2f1907a4bebd712981a70b8c62353113ce365c7b
Author: Jamal Hadi Salim <jhs@mojatatu.com>

net/sched: Handle TC_ACT_REDIRECT from qdisc filter chains

When a TC filter attached to a qdisc filter chain returns TC_ACT_REDIRECT,
the redirect was silently lost. This patch adds a tcf_classify_qdisc()
wrapper to convert TC_ACT_REDIRECT to TC_ACT_SHOT, ensuring the packet is
dropped rather than silently passed through.

> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index 3bd08d7f39c18..5f5cb36439fe2 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h

[ ... ]

> @@ -156,8 +156,20 @@ static inline int tcf_classify(struct sk_buff *skb,
>  {
>  	return TC_ACT_UNSPEC;
>  }
> -
> -#endif
> +static inline int tcf_classify_qdisc(struct sk_buff *skb,
> +				     const struct tcf_proto *tp,
> +				     struct tcf_result *res, bool compat_mode)
> +{
> +	int ret = tcf_classify(skb, NULL, tp, res, compat_mode);
> +
> +	/* TC_ACT_REDIRECT from qdisc filter chains is not supported.
> +	 * Use BPF via tcx or mirred redirect instead.
> +	 */
> +	if (unlikely(ret == TC_ACT_REDIRECT))

[Severity: High]
This is a pre-existing issue, but since this patch explicitly discusses
handling TC_ACT_REDIRECT verdicts, does the current clsact fast path leak
uninitialized kernel stack memory?

If a user attaches a TC BPF program that simply returns TC_ACT_REDIRECT
without calling the bpf_redirect() helper, it appears we use uninitialized
stack data.

In net/core/dev.c:sch_handle_ingress(), the bpf_net_context is allocated on
the stack without initialization:

    struct bpf_net_context __bpf_net_ctx, *bpf_net_ctx;
    ...
    bpf_net_ctx = bpf_net_ctx_set(&__bpf_net_ctx);

When include/linux/filter.h:bpf_net_ctx_set() is called, it only zeroes
kern_flags:

    bpf_net_ctx->ri.kern_flags = 0;

This leaves the rest of the redirect info (ri.tgt_index, ri.flags, and
ri.nh) uninitialized. 

Later, when net/core/filter.c:skb_do_redirect() is called to process the
TC_ACT_REDIRECT verdict:

    struct bpf_redirect_info *ri = bpf_net_ctx_get_ri();
    ...
    u32 flags = ri->flags;
    dev = dev_get_by_index_rcu(net, ri->tgt_index);

It reads this uninitialized stack memory. If the flags happen to have
BPF_F_NEIGH set, the uninitialized ri->nh structure is used to generate an
ARP/NDP request, potentially leaking kernel stack memory on the network.

Should bpf_net_ctx_set() or sch_handle_ingress() zero out the entire
__bpf_net_ctx structure to prevent this leak?

> +		ret = TC_ACT_SHOT;
> +	return ret;
> +}
>  
>  static inline unsigned long
>  __cls_set_class(unsigned long *clp, unsigned long cl)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706185609.330006-1-daniel@iogearbox.net?part=2