Re: [PATCH net-next v2] net: check qdisc_pkt_len_segs_init() return value on ingress

[Daniel Borkmann <daniel@iogearbox.net>]

On 4/13/26 8:22 PM, David Carlier wrote:
> Commit 7fb4c1967011 ("net: pull headers in qdisc_pkt_len_segs_init()")
> changed qdisc_pkt_len_segs_init() to return an skb drop reason when
> it detects malicious GSO packets. The egress path in __dev_queue_xmit()
> checks this return value and drops bad packets, but the ingress path in
> sch_handle_ingress() ignores it.
> 
> This means malformed GSO packets entering via TC ingress are not dropped
> and could be redirected to another interface or cause incorrect qdisc
> accounting.

Why we need to do this on both sides (and what's the perf impact)? If TC
ingress redirects it to some other device, then don't we hit the same via
__dev_queue_xmit() where the 7fb4c1967011 added the qdisc_pkt_len_segs_init()?

> Check the return value and drop the packet when a bad GSO is detected.
> 
> Fixes: 7fb4c1967011 ("net: pull headers in qdisc_pkt_len_segs_init()")
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
> 
> v1 -> v2: reorder variable declarations for reverse xmas tree
> v1: https://lore.kernel.org/netdev/20260408172307.46498-1-devnexen@gmail.com/
>   net/core/dev.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 5a31f9d2128c..d11c22cafca9 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4459,8 +4459,8 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
>   		   struct net_device *orig_dev, bool *another)
>   {
>   	struct bpf_mprog_entry *entry = rcu_dereference_bh(skb->dev->tcx_ingress);
> -	enum skb_drop_reason drop_reason = SKB_DROP_REASON_TC_INGRESS;
>   	struct bpf_net_context __bpf_net_ctx, *bpf_net_ctx;
> +	enum skb_drop_reason drop_reason;
>   	int sch_ret;
>   
>   	if (!entry)
> @@ -4472,7 +4472,15 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
>   		*pt_prev = NULL;
>   	}
>   
> -	qdisc_pkt_len_segs_init(skb);
> +	drop_reason = qdisc_pkt_len_segs_init(skb);
> +	if (unlikely(drop_reason)) {
> +		kfree_skb_reason(skb, drop_reason);
> +		*ret = NET_RX_DROP;
> +		bpf_net_ctx_clear(bpf_net_ctx);
> +		return NULL;
> +	}
> +
> +	drop_reason = SKB_DROP_REASON_TC_INGRESS;
>   	tcx_set_ingress(skb, true);
>   
>   	if (static_branch_unlikely(&tcx_needed_key)) {