public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: check qdisc_pkt_len_segs_init() return value on ingress
@ 2026-04-08 17:23 David Carlier
  2026-04-12 18:21 ` Jakub Kicinski
  0 siblings, 1 reply; 2+ messages in thread
From: David Carlier @ 2026-04-08 17:23 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, horms
  Cc: sdf, kuniyu, skhawaja, liuhangbin, krikku, netdev, linux-kernel,
	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>
---
 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..2b5f508fc479 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4459,7 +4459,7 @@ 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;
+	enum skb_drop_reason drop_reason;
 	struct bpf_net_context __bpf_net_ctx, *bpf_net_ctx;
 	int sch_ret;
 
@@ -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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] net: check qdisc_pkt_len_segs_init() return value on ingress
  2026-04-08 17:23 [PATCH] net: check qdisc_pkt_len_segs_init() return value on ingress David Carlier
@ 2026-04-12 18:21 ` Jakub Kicinski
  0 siblings, 0 replies; 2+ messages in thread
From: Jakub Kicinski @ 2026-04-12 18:21 UTC (permalink / raw)
  To: David Carlier
  Cc: davem, edumazet, pabeni, horms, sdf, kuniyu, skhawaja, liuhangbin,
	krikku, netdev, linux-kernel

On Wed,  8 Apr 2026 18:23:07 +0100 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.
> 
> 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>

Not sure this can happen today, but okay.
Hopefully we won't get a patch for every Sashiko report we knowingly
ignored :|

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 5a31f9d2128c..2b5f508fc479 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4459,7 +4459,7 @@ 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;
> +	enum skb_drop_reason drop_reason;

this needs to move one line down now to keep the variable ordering.

>  	struct bpf_net_context __bpf_net_ctx, *bpf_net_ctx;
>  	int sch_ret;
>  
> @@ -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)) {
-- 
pw-bot: cr

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-04-12 18:21 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-08 17:23 [PATCH] net: check qdisc_pkt_len_segs_init() return value on ingress David Carlier
2026-04-12 18:21 ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox