All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/3] net/sched: fix short-skb metadata and namespace boundaries
@ 2026-10-29 23:27 Jamal Hadi Salim
  2026-10-29 23:27 ` [PATCH net 1/3] net/sched: act_ife: encode configured zero metadata values Jamal Hadi Salim
  2026-10-29 23:59 ` [PATCH net 0/3] net/sched: fix short-skb metadata and namespace boundaries Jakub Kicinski
  0 siblings, 2 replies; 6+ messages in thread
From: Jamal Hadi Salim @ 2026-10-29 23:27 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, stable, shaos, David S . Miller, Eric Dumazet,
	Simon Horman, Jiri Pirko, Jakub Kicinski, Paolo Abeni,
	Victor Nogueira, Sashiko, Vega

This series fixes three independent boundary problems in net/sched.

1/3 act_ife: encode a configured zero metadata value.  ife_check_meta_u32()
reserves an 8-byte TLV for a configured zero value, but the encoder
returned without writing it, leaking stale skb headroom bytes.

2/3 act_ife: require a full Ethernet header on egress.  An undersized
linear head can make the encoder read past the end of the skb data.

3/3 sch_teql: reject qdiscs from another network namespace.  The global
TEQL master lives in the initial netns; a qdisc created in another netns
could still mutate the master's MTU and flags.

Origins:

1/3: reported by shaos <shawnq1@163.com> with a reproducer, which was
reproduced and verified in our test environment.
2/3: found by Sashiko (nipa and gemini) while reviewing Victor Nogueira's
parent act_ife patchset; independently reported by shaos
<shawnq1@163.com> with a reproducer, which was verified in our test
environment.
3/3: reported by shaos <shawnq1@163.com> with a reproducer, which was
reproduced and verified in our test environment.

The three patches are independent.  They target net and carry a stable
nomination.  They are sent together because they share the short-skb and
namespace-boundary review context.

Jamal Hadi Salim (3):
  net/sched: act_ife: encode a configured zero metadata value
  net/sched: act_ife: require a full Ethernet header on egress
  net/sched: sch_teql: reject qdiscs from another network namespace

 net/sched/act_ife.c  | 8 +++++---
 net/sched/sch_teql.c | 5 +++++
 2 files changed, 10 insertions(+), 3 deletions(-)

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---

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

* [PATCH net 1/3] net/sched: act_ife: encode configured zero metadata values
  2026-10-29 23:27 [PATCH net 0/3] net/sched: fix short-skb metadata and namespace boundaries Jamal Hadi Salim
@ 2026-10-29 23:27 ` Jamal Hadi Salim
  2026-10-29 23:27   ` [PATCH net 2/3] net/sched: act_ife: require a full Ethernet header on egress Jamal Hadi Salim
  2026-10-29 23:59 ` [PATCH net 0/3] net/sched: fix short-skb metadata and namespace boundaries Jakub Kicinski
  1 sibling, 1 reply; 6+ messages in thread
From: Jamal Hadi Salim @ 2026-10-29 23:27 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, stable, shaos, Jiri Pirko, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Victor Nogueira

Reported by shaos <shawnq1@163.com> with a reproducer for the 32-bit
case, which was reproduced and verified in our test environment.

The issue:
ife_check_meta_u32() and ife_check_meta_u16() both reserve eight bytes
of metadata for a configured value even when that value is zero, but the
matching encoder returns without calling ife_tlv_meta_encode(). The
encoder therefore writes neither the TLV header nor its value, while the
enclosing IFE length still includes the reserved bytes, exposing stale
skb headroom contents to the peer.

The 32-bit helper has been present since the IFE action was introduced
(v4.6); the 16-bit helper was added with the 16-bit metadata helpers used
by the skbtcindex callback (v4.9). Both defects predate the oldest
supported stable kernel (5.10), so they are fixed together in this patch.
Each hunk keeps its own Fixes: tag for provenance.

The fix:
Encode any configured value, including zero; skip the TLV only when
there is neither a configured nor a runtime value. The runtime fallback
is unchanged.

A raw TCA_IFE_METALST netlink request is needed to express the
configured zero: the IFE_META_SKBMARK attribute carries four zero
payload bytes for the u32 helper, and IFE_META_TCINDEX two zero payload
bytes for the 16-bit helper (not an absent or zero-length attribute).
Stock tc cannot build these requests; "use mark 0x0" emits no mark
attribute and "allow mark" emits a zero-length one.

Conditions to recreate the bug:
CONFIG_NET_ACT_IFE=y, CONFIG_NET_IFE_SKBMARK=y,
CONFIG_NET_IFE_SKBTCINDEX=y, CONFIG_NET_CLS_MATCHALL=y,
CONFIG_NET_SCH_INGRESS=y, CONFIG_USER_NS=y, CONFIG_NET_NS=y,
CONFIG_VETH=y. In a fresh user and network namespace create a veth pair
and attach clsact to the egress side. Install a matchall filter whose
ife encode action carries a zero skb mark (or a zero tcindex) in
TCA_IFE_METALST. Send a frame through the veth and inspect the IFE
metadata in the received frame. The metadata region is eight bytes but
holds neither a valid TLV header nor a value; it exposes prior headroom
contents. Setting the value to a nonzero one does not trigger the leak.
The namespace needs CAP_NET_ADMIN; no additional privilege is required.
With the fix the zero TLVs are 0001000800000000 (u32) and
0005000600000000 (u16).

Fixes: ef6980b6becb ("introduce IFE action")
Fixes: 6a5d58b67e20 ("net sched ife action: add 16 bit helpers")
Reported-by: shaos <shawnq1@163.com>
Link: https://lore.kernel.org/netdev/20261022144739.61342-1-shawnq1@163.com/
Cc: stable@vger.kernel.org
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/act_ife.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 2afd68983ece..0e6aba9aeff1 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -50,8 +50,7 @@ int ife_encode_meta_u16(u16 metaval, void *skbdata, struct tcf_meta_info *mi)
 		edata = *(u16 *)mi->metaval;
 	else if (metaval)
 		edata = metaval;
-
-	if (!edata) /* will not encode */
+	else
 		return 0;
 
 	edata = htons(edata);
@@ -94,8 +93,7 @@ int ife_encode_meta_u32(u32 metaval, void *skbdata, struct tcf_meta_info *mi)
 		edata = *(u32 *)mi->metaval;
 	else if (metaval)
 		edata = metaval;
-
-	if (!edata) /* will not encode */
+	else
 		return 0;
 
 	edata = htonl(edata);
-- 
2.43.0


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

* [PATCH net 2/3] net/sched: act_ife: require a full Ethernet header on egress
  2026-10-29 23:27 ` [PATCH net 1/3] net/sched: act_ife: encode configured zero metadata values Jamal Hadi Salim
@ 2026-10-29 23:27   ` Jamal Hadi Salim
  2026-10-29 23:27     ` [PATCH net 3/3] net/sched: sch_teql: reject qdiscs from another network namespace Jamal Hadi Salim
  0 siblings, 1 reply; 6+ messages in thread
From: Jamal Hadi Salim @ 2026-10-29 23:27 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, stable, shaos, David S . Miller, Eric Dumazet,
	Simon Horman, Jiri Pirko, Jakub Kicinski, Paolo Abeni,
	Victor Nogueira, Sashiko, Vega

This is a followup of commit 5b483f7791b0 ("net/sched: act_ife: Only
operate on Ethernet frames") as reported by Sashiko.

The finding was raised by both Sashiko providers (nipa and gemini)
while reviewing the parent patchset. shaos <shawnq1@163.com>
independently reported the same undersized-egress path with a
reproducer, which was reproduced and verified in our test environment.

The issue:
On egress tcf_ife_is_eth_skb() only checks the device type; it does not
check that the skb carries a full Ethernet header. For an Ethernet or
loopback device it returns true even when the skb has no Ethernet
header. tcf_ife_encode() then calls ife_encode(), which copies ETH_HLEN
bytes from skb->data. A short skb therefore reads past its linear data
and the copied bytes, including stale kernel memory, are emitted in the
outer Ethernet header.

The parent commit fixed the reported ingress mac_len case but left the
egress path without a length check.

A total-length test is not sufficient: a non-GSO TUN skb can have a
large total length while only a single byte sits in the linear area, so
ife_encode() still reads past the linear header. Require instead that
the Ethernet header the encoder copies is present in the linear part of
the skb.

The Fix:
On egress require pskb_may_pull(skb, ETH_HLEN) before encoding, which
ensures the Ethernet header is present in the linear data ife_encode()
copies from; the ingress mac_len check is unchanged. A short egress skb
is now dropped instead of being encoded.

Conditions to recreate the bug:
CONFIG_NET_ACT_IFE=y, CONFIG_NET_CLS_MATCHALL=y, CONFIG_NET_SCH_INGRESS=y,
CONFIG_TUN=y, CONFIG_NET_ACT_MIRRED=y. In two user and network
namespaces (a victim and an attacker), open a TUN device in IFF_TUN mode
and attach clsact to it and to lo. Redirect ingress traffic from the TUN
device to lo egress with mirred, and install an ife encode action on lo
egress:

  tc qdisc add dev <tun> clsact
  tc qdisc add dev lo clsact
  tc filter add dev <tun> ingress matchall action mirred egress \
      redirect dev lo
  tc filter add dev lo egress matchall action ife encode use mark 1 \
      action ok

Send a TUN packet shorter than ETH_HLEN (a one-byte payload is enough)
and receive the emitted frame on the other side. Without the fix the
outer Ethernet header contains bytes copied past the short packet; with
the fix the short skb is dropped. The attacker needs CAP_NET_ADMIN in
its user namespace; no init-net privilege is required.

The same path is reachable when the total length is large but the
Ethernet header is not linear: write a non-GSO virtio header with
hdr_len=1 followed by an 8192-byte packet so that only one byte stays
linear. Without the fix the copied header reads past the linear data;
with the fix the header is pulled into the linear area before the copy.

Fixes: ef6980b6becb ("introduce IFE action")
Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org>
Reported-by: shaos <shawnq1@163.com>
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260821164031.32824-1-victor%40mojatatu.com
Link: https://sashiko.dev/#/patchset/20260821164031.32824-1-victor@mojatatu.com
Link: https://lore.kernel.org/netdev/20261022144736.61227-1-shawnq1@163.com/
Cc: stable@vger.kernel.org
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/act_ife.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 2afd68983ece..986264af8146 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -876,13 +876,16 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
  * is an Ethernet header because the skb could've been redirected with mirred
  * from a non-Ethernet device.
  */
-static bool tcf_ife_is_eth_skb(const struct sk_buff *skb)
+static bool tcf_ife_is_eth_skb(struct sk_buff *skb)
 {
 	if (skb->dev->type != ARPHRD_ETHER &&
 	    skb->dev->type != ARPHRD_LOOPBACK)
 		return false;
 
-	return !skb_at_tc_ingress(skb) || skb->mac_len == ETH_HLEN;
+	if (skb_at_tc_ingress(skb))
+		return skb->mac_len == ETH_HLEN;
+
+	return pskb_may_pull(skb, ETH_HLEN);
 }
 
 TC_INDIRECT_SCOPE int tcf_ife_act(struct sk_buff *skb,
-- 
2.43.0


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

* [PATCH net 3/3] net/sched: sch_teql: reject qdiscs from another network namespace
  2026-10-29 23:27   ` [PATCH net 2/3] net/sched: act_ife: require a full Ethernet header on egress Jamal Hadi Salim
@ 2026-10-29 23:27     ` Jamal Hadi Salim
  0 siblings, 0 replies; 6+ messages in thread
From: Jamal Hadi Salim @ 2026-10-29 23:27 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, stable, shaos, David S . Miller, Eric Dumazet,
	Simon Horman, Jiri Pirko, Jakub Kicinski, Paolo Abeni,
	Victor Nogueira, Tom Goff

Reported by shaos <shawnq1@163.com> with a reproducer, which was
reproduced and verified in our test environment.

The issue:
The teql master devices are created once in the initial network
namespace and their Qdisc_ops are global. teql_qdisc_init() takes the
teql_master embedded in the registered sch->ops and uses it without
checking whether the new qdisc's device lives in the same network
namespace as the master. A qdisc created in another netns can therefore
attach to the init-net master and change its MTU and flags.

The Fix:
Reject a teql qdisc whose device is not in the master's network
namespace with -EOPNOTSUPP and an extack message; same-netns teql still
works.

Initialize the private skb queue before any validation so every
constructor failure path, including qdisc_create_dflt(), which resets the
qdisc before destroying it, sees an initialized empty queue.

Conditions to recreate the bug:
CONFIG_NET_SCH_TEQL=y, CONFIG_DUMMY=y, CONFIG_USER_NS=y, CONFIG_NET_NS=y.
As an unprivileged UID, unshare a user and network namespace, create a
dummy device, set its MTU to 60000, and add a root teql0 qdisc:

  unshare -Urn
  ip link add dummy0 type dummy
  ip link set dummy0 mtu 60000
  tc qdisc add dev dummy0 root teql0

The init-net teql0 device changes from MTU 1500 to 60000. A pfifo control
on the same device leaves the MTU unchanged, and a direct MTU change on
init-net teql0 is denied. The namespace needs CAP_NET_ADMIN; no init-net
privilege is required. With the fix the attach fails with
"Operation not supported" and the init-net MTU stays 1500.

Fixes: 7316ae88c43d ("net_sched: make traffic control network namespace aware")
Reported-by: shaos <shawnq1@163.com>
Link: https://lore.kernel.org/netdev/20261022144830.67948-1-shawnq1@163.com/
Cc: stable@vger.kernel.org
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_teql.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 409ce50cc0db..55f9944ca47d 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -188,12 +188,19 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
 	struct teql_sched_data *q = qdisc_priv(sch);
 	struct Qdisc *first;
 
+	skb_queue_head_init(&q->q);
+
 	if (dev->hard_header_len > m->dev->hard_header_len)
 		return -EINVAL;
 
 	if (m->dev == dev)
 		return -ELOOP;
 
+	if (dev_net(dev) != dev_net(m->dev)) {
+		NL_SET_ERR_MSG_MOD(extack, "teql master is in a different network namespace");
+		return -EOPNOTSUPP;
+	}
+
 	if (sch->parent != TC_H_ROOT) {
 		NL_SET_ERR_MSG_MOD(extack, "teql can only be used as root");
 		return -EOPNOTSUPP;
@@ -201,8 +208,6 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
 
 	q->m = m;
 
-	skb_queue_head_init(&q->q);
-
 	spin_lock_bh(&m->slaves_lock);
 	first = rcu_dereference_protected(m->slaves, lockdep_is_held(&m->slaves_lock));
 	if (first) {
-- 
2.34.1


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

* Re: [PATCH net 0/3] net/sched: fix short-skb metadata and namespace boundaries
  2026-10-29 23:27 [PATCH net 0/3] net/sched: fix short-skb metadata and namespace boundaries Jamal Hadi Salim
  2026-10-29 23:27 ` [PATCH net 1/3] net/sched: act_ife: encode configured zero metadata values Jamal Hadi Salim
@ 2026-10-29 23:59 ` Jakub Kicinski
  2026-10-30  0:07   ` Jamal Hadi Salim
  1 sibling, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2026-10-29 23:59 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev, stable, shaos, David S . Miller, Eric Dumazet,
	Simon Horman, Jiri Pirko, Paolo Abeni, Victor Nogueira, Sashiko,
	Vega

On Thu, 29 Oct 2026 19:27:36 -0400 Jamal Hadi Salim wrote:
> Subject: [PATCH net 0/3] net/sched: fix short-skb metadata and namespace boundaries

Please fix your setup, Jamal, all your recent postings have patches
posted in reply to the previous patch:

Patch 2 is in-reply-to patch 1, patch 3 replying to patch 2.
They should all be in-reply-to the cover letter.

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

* Re: [PATCH net 0/3] net/sched: fix short-skb metadata and namespace boundaries
  2026-10-29 23:59 ` [PATCH net 0/3] net/sched: fix short-skb metadata and namespace boundaries Jakub Kicinski
@ 2026-10-30  0:07   ` Jamal Hadi Salim
  0 siblings, 0 replies; 6+ messages in thread
From: Jamal Hadi Salim @ 2026-10-30  0:07 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, stable, shaos, David S . Miller, Eric Dumazet,
	Simon Horman, Jiri Pirko, Paolo Abeni, Victor Nogueira, Sashiko,
	Vega

On Thu, Oct 29, 2026 at 7:59 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 29 Oct 2026 19:27:36 -0400 Jamal Hadi Salim wrote:
> > Subject: [PATCH net 0/3] net/sched: fix short-skb metadata and namespace boundaries
>
> Please fix your setup, Jamal, all your recent postings have patches
> posted in reply to the previous patch:
>
> Patch 2 is in-reply-to patch 1, patch 3 replying to patch 2.
> They should all be in-reply-to the cover letter.

I know this is what you asked for before - but then Sashiko started
getting weird and i went backwards and saw it accepted this format. I
will resend this in 24 hours and stick to having it reply to patch 0.
Maybe the sashiko issue is not even related to this but it was getting
a little frustrating. In any case , sorry ;->

cheers,
jamal

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

end of thread, other threads:[~2026-10-30  0:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-29 23:27 [PATCH net 0/3] net/sched: fix short-skb metadata and namespace boundaries Jamal Hadi Salim
2026-10-29 23:27 ` [PATCH net 1/3] net/sched: act_ife: encode configured zero metadata values Jamal Hadi Salim
2026-10-29 23:27   ` [PATCH net 2/3] net/sched: act_ife: require a full Ethernet header on egress Jamal Hadi Salim
2026-10-29 23:27     ` [PATCH net 3/3] net/sched: sch_teql: reject qdiscs from another network namespace Jamal Hadi Salim
2026-10-29 23:59 ` [PATCH net 0/3] net/sched: fix short-skb metadata and namespace boundaries Jakub Kicinski
2026-10-30  0:07   ` 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.