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

Daniel Borkmann
2026-04-13T18:38:34Z

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