All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 1/2 repost1] net/sched: sch_teql: fix shared headroom and header strip on slave retry
@ 2026-10-12 18:16 Jamal Hadi Salim
  2026-10-20  2:40 ` Jakub Kicinski
  0 siblings, 1 reply; 3+ messages in thread
From: Jamal Hadi Salim @ 2026-10-12 18:16 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, Victor Nogueira, Jiri Pirko, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	sashiko-bot, stable

teql_master_xmit() writes the link header of the current slave directly
into the skb via teql_resolve(), and on retry undoes it with
__skb_pull(skb, skb_network_offset(skb)).  Two problems exist on the
retry path:

1. When the skb is shared (for example a packet tap installed on the
   master makes xmit_one() clone it before teql_master_xmit()),
   dev_hard_header() writes into memory shared with the other clones and
   corrupts them.  The headroom must be made private before the write.

2. The pull blindly removes skb_network_offset() bytes.  For a frame
   that carries its own link header in the payload (AF_PACKET/SOCK_RAW
   with no dst entry), teql_resolve() returns 0 without adding a header,
   so the retry strips the userspace-supplied header and pushes a frame
   whose first bytes are payload.

Fix this by calling skb_cow_head() before dev_hard_header(), and by
pulling back only the header length that this slave actually added,
tracked per iteration.  An AF_PACKET/SOCK_RAW frame is then retried on
the next slave with its header intact, and a shared skb is unshared
before it is modified.

This is a follow-up to commit dc4b95b8fee9 ("net/sched: sch_teql: restore
skb->dev on the slave failure path"), which restored skb->dev to the
master on the slave failure path but left these pre-existing defects on
the same teql_resolve()/retry path.

Conditions to recreate the bug: a teql master with at least two slaves,
the first of which does not consume the skb, then send an
AF_PACKET/SOCK_RAW frame (no dst entry) through the master: the frame
arriving on the second slave's peer has its MAC header stripped.  The
shared-headroom write needs a packet tap on the master so the skb is
cloned before teql_master_xmit().

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260824115928.4099988-1-victor@mojatatu.com
Link: https://lore.kernel.org/netdev/20260824115928.4099988-1-victor@mojatatu.com/
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_teql.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 409ce50cc0db..acd03f9afc6b 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -245,7 +245,7 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
 static int
 __teql_resolve(struct sk_buff *skb, struct sk_buff *skb_res,
 	       struct net_device *dev, struct netdev_queue *txq,
-	       struct dst_entry *dst)
+	       struct dst_entry *dst, int *hlen)
 {
 	struct neighbour *n;
 	int err = 0;
@@ -265,15 +265,28 @@ __teql_resolve(struct sk_buff *skb, struct sk_buff *skb_res,
 	}
 
 	if (neigh_event_send(n, skb_res) == 0) {
+		int off = skb_network_offset(skb);
 		char haddr[MAX_ADDR_LEN];
 
 		neigh_ha_snapshot(haddr, n, dev);
+		/* The skb may be shared (e.g. a packet tap clone); make the
+		 * headroom private before dev_hard_header() writes into it.
+		 */
+		if (skb_cow_head(skb, LL_RESERVED_SPACE(dev)) < 0) {
+			err = -ENOMEM;
+			goto out;
+		}
 		if (dev_hard_header(skb, dev, ntohs(skb_protocol(skb, false)),
 				    haddr, NULL, skb->len) < 0)
 			err = -EINVAL;
+		/* The header, if any, is prepended above skb->data, so the
+		 * network offset grew by exactly the bytes to undo later.
+		 */
+		*hlen = skb_network_offset(skb) - off;
 	} else {
 		err = (skb_res == NULL) ? -EAGAIN : 1;
 	}
+out:
 	neigh_release(n);
 	return err;
 }
@@ -281,11 +294,13 @@ __teql_resolve(struct sk_buff *skb, struct sk_buff *skb_res,
 static inline int teql_resolve(struct sk_buff *skb,
 			       struct sk_buff *skb_res,
 			       struct net_device *dev,
-			       struct netdev_queue *txq)
+			       struct netdev_queue *txq,
+			       int *hlen)
 {
 	struct dst_entry *dst = skb_dst(skb);
 	int res;
 
+	*hlen = 0;
 	if (rcu_access_pointer(txq->qdisc) == &noop_qdisc)
 		return -ENODEV;
 
@@ -293,7 +308,7 @@ static inline int teql_resolve(struct sk_buff *skb,
 		return 0;
 
 	rcu_read_lock();
-	res = __teql_resolve(skb, skb_res, dev, txq, dst);
+	res = __teql_resolve(skb, skb_res, dev, txq, dst, hlen);
 	rcu_read_unlock();
 
 	return res;
@@ -305,6 +320,7 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct Qdisc *start, *q;
 	int busy;
 	int nores;
+	int hlen;
 	int subq = skb_get_queue_mapping(skb);
 	struct sk_buff *skb_res = NULL;
 
@@ -332,7 +348,7 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
 			continue;
 		}
 
-		switch (teql_resolve(skb, skb_res, slave, slave_txq)) {
+		switch (teql_resolve(skb, skb_res, slave, slave_txq, &hlen)) {
 		case 0:
 			if (__netif_tx_trylock(slave_txq)) {
 				unsigned int length = qdisc_pkt_len(skb);
@@ -374,8 +390,14 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
 			nores = 1;
 			break;
 		}
+		/* Undo only the header teql_resolve() pushed for this slave.
+		 * Pulling skb_network_offset() instead would strip a
+		 * userspace-supplied header when the slave added none, e.g.
+		 * an AF_PACKET/SOCK_RAW frame with no dst entry.
+		 */
+		if (hlen > 0)
+			__skb_pull(skb, hlen);
 		skb->dev = dev;
-		__skb_pull(skb, skb_network_offset(skb));
 	} while ((q = rcu_dereference(NEXT_SLAVE(q))) != start);
 
 	if (nores && skb_res == NULL) {
-- 
2.43.0


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

* Re: [PATCH net 1/2 repost1] net/sched: sch_teql: fix shared headroom and header strip on slave retry
  2026-10-12 18:16 [PATCH net 1/2 repost1] net/sched: sch_teql: fix shared headroom and header strip on slave retry Jamal Hadi Salim
@ 2026-10-20  2:40 ` Jakub Kicinski
  2026-10-20 10:26   ` Jamal Hadi Salim
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2026-10-20  2:40 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev, Victor Nogueira, Jiri Pirko, David S . Miller,
	Eric Dumazet, Paolo Abeni, Simon Horman, sashiko-bot, stable

On Mon, 12 Oct 2026 14:16:27 -0400 Jamal Hadi Salim wrote:
> Subject: [PATCH net 1/2 repost1] net/sched: sch_teql: fix shared headroom and header strip on slave retry

not correctly threaded, again..
patch 2 should be in reply to patch 1
-- 
pw-bot: cr

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

* Re: [PATCH net 1/2 repost1] net/sched: sch_teql: fix shared headroom and header strip on slave retry
  2026-10-20  2:40 ` Jakub Kicinski
@ 2026-10-20 10:26   ` Jamal Hadi Salim
  0 siblings, 0 replies; 3+ messages in thread
From: Jamal Hadi Salim @ 2026-10-20 10:26 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, Victor Nogueira, Jiri Pirko, David S . Miller,
	Eric Dumazet, Paolo Abeni, Simon Horman, sashiko-bot, stable

On Mon, Oct 19, 2026 at 10:40 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 12 Oct 2026 14:16:27 -0400 Jamal Hadi Salim wrote:
> > Subject: [PATCH net 1/2 repost1] net/sched: sch_teql: fix shared headroom and header strip on slave retry
>
> not correctly threaded, again..
> patch 2 should be in reply to patch 1

Thanks for catching it - will resend. I fixed the scripts so this
doesnt happen again, but looks like this one went out before i did
that.

cheers,
jamal

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

end of thread, other threads:[~2026-10-20 10:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 18:16 [PATCH net 1/2 repost1] net/sched: sch_teql: fix shared headroom and header strip on slave retry Jamal Hadi Salim
2026-10-20  2:40 ` Jakub Kicinski
2026-10-20 10:26   ` Jamal Hadi Salim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.