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

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

[David Carlier]

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.

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

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

[Daniel Borkmann]

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

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

[Eric Dumazet]

On Mon, Apr 13, 2026 at 11:22 AM David Carlier <devnexen@gmail.com> 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.
>
> 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(-)

NACK. This is is not needed.

I will not even bother, this is an obvious LLM based patch.

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

[David CARLIER]

Hi Eric,

  You're right on both counts. An LLM was used for the initial
bug-finding /
  pre-analysis on this one, and clearly it wasn't deep enough.
Daniel's
  question made me go back and trace the redirect paths properly — the
  premise was wrong, __dev_queue_xmit() already catches them. I should
have
  re-read your 7fb4c1967011 commit message before sending.

  Dropping it. Sorry for the noise.

  Cheers,

On Mon, 13 Apr 2026 at 21:09, Eric Dumazet <edumazet@google.com> wrote:
>
> On Mon, Apr 13, 2026 at 11:22 AM David Carlier <devnexen@gmail.com> 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.
> >
> > 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(-)
>
> NACK. This is is not needed.
>
> I will not even bother, this is an obvious LLM based patch.