Linux kernel -stable discussions
 help / color / mirror / Atom feed
* [PATCH net v3 0/4] net/sched: fix short-skb metadata and namespace boundaries
@ 2026-11-03 11:50 Jamal Hadi Salim
  2026-11-03 11:50 ` [PATCH net v3 1/4] net/sched: act_ife: encode configured zero metadata values Jamal Hadi Salim
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Jamal Hadi Salim @ 2026-11-03 11:50 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, Tom Goff, Shuah Khan, Yotam Gigi,
	linux-kselftest

This series fixes three independent boundary problems in net/sched, and
covers the third one with tc-testing cases.

1/4 act_ife: encode a configured zero metadata value.  The u32 and u16
presence callbacks reserve an 8-byte TLV for a configured zero value, but
the encoders returned without writing it, leaking stale skb headroom bytes.

2/4 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/4 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.

4/4 selftests: tc-testing: cover each of the two teql attach rejections on
its own.  Case 124e moves to the initial namespace, where it creates its
own device, so only the root-only check can refuse it; a root attach from
the test namespace is refused only by the namespace check.

Origins:

1/4: reported by shaos <shawnq1@163.com> with a reproducer, which was
reproduced and verified in our test environment.
2/4: 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/4: reported by shaos <shawnq1@163.com> with a reproducer, which was
reproduced and verified in our test environment.
4/4: follows Sashiko (nipa) reviews of the v2 posting, which pointed out
that a single case cannot witness both teql attach rejections.

The four patches are independent.  They target net and the three fixes
carry a stable nomination; the test cases do not.  They are sent together
because they share the short-skb and namespace-boundary review context.

Jamal Hadi Salim (4):
  net/sched: act_ife: encode configured zero metadata values
  net/sched: act_ife: require a full Ethernet header on egress
  net/sched: sch_teql: reject qdiscs from another network namespace
  selftests: tc-testing: teql: cover each attach rejection on its own

 net/sched/act_ife.c                                | 13 ++++----
 net/sched/sch_teql.c                               |  9 ++++--
 .../selftests/tc-testing/tc-tests/qdiscs/teql.json | 36 ++++++++++++++++++----
 3 files changed, 44 insertions(+), 14 deletions(-)

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

v2 -> v3: considers comments from Sashiko (nipa) on the v2 posting.  Case 124e
ran under nsPlugin, in a namespace where the teql0 master is not present,
so the namespace check answered it and the case no longer proved that a
child attach is refused as such; the case added in v2 covered only the
namespace check.  124e now runs in the initial namespace and creates its
own device, and the namespace rejection keeps its own case.  Both cases
were verified to fail when the check they exercise is removed.

1/4's and 3/4's diffs are unchanged from the v2 posting, so neither carries
a changelog; 2/4's Fixes: tag now names the commit that introduced the
unguarded copy.

v1 -> v2: considers comments from Sashiko (nipa) on the repost2 posting.  The
cross-netns check in 3/4 ran before the "teql can only be used as root"
check, so every case it did not catch was answered by that check, masking
the assertion that tc-testing case 124e makes.  The root-parent validation
now runs first and the namespace check second, and 4/4 covers the namespace
rejection on its own as suggested by sashiko-nipa.

1/4 and 2/4 are unchanged from the repost2 posting.

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

* [PATCH net v3 1/4] net/sched: act_ife: encode configured zero metadata values
  2026-11-03 11:50 [PATCH net v3 0/4] net/sched: fix short-skb metadata and namespace boundaries Jamal Hadi Salim
@ 2026-11-03 11:50 ` Jamal Hadi Salim
  2026-11-09 16:20   ` Simon Horman
  2026-11-03 11:50 ` [PATCH net v3 2/4] net/sched: act_ife: require a full Ethernet header on egress Jamal Hadi Salim
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Jamal Hadi Salim @ 2026-11-03 11:50 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 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
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
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 3946a9589bb8..6b23a3965121 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] 12+ messages in thread

* [PATCH net v3 2/4] net/sched: act_ife: require a full Ethernet header on egress
  2026-11-03 11:50 [PATCH net v3 0/4] net/sched: fix short-skb metadata and namespace boundaries Jamal Hadi Salim
  2026-11-03 11:50 ` [PATCH net v3 1/4] net/sched: act_ife: encode configured zero metadata values Jamal Hadi Salim
@ 2026-11-03 11:50 ` Jamal Hadi Salim
  2026-11-07 14:50   ` netdev-bot+sashiko
  2026-11-03 11:50 ` [PATCH net v3 3/4] net/sched: sch_teql: reject qdiscs from another network namespace Jamal Hadi Salim
  2026-11-10 18:23 ` [PATCH net v3 0/4] net/sched: fix short-skb metadata and namespace boundaries patchwork-bot+netdevbpf
  3 siblings, 1 reply; 12+ messages in thread
From: Jamal Hadi Salim @ 2026-11-03 11:50 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, Yotam Gigi

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: 295a6e06d21e ("net/sched: act_ife: Change to use ife module")
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>
---
v2 -> v3: no change to the diff; the Fixes: tag now names the commit that
introduced the unguarded copy.  This series revision considers comments
from Sashiko (nipa) on the v2 posting:
https://lore.kernel.org/netdev/179366641863.488856.17368571591466000274@kernel.org/

The tc-testing coverage of the teql rejections is reworked in 4/4 so that
each case exercises exactly one check.

 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 6b23a3965121..5a527da00566 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -878,13 +878,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] 12+ messages in thread

* [PATCH net v3 3/4] net/sched: sch_teql: reject qdiscs from another network namespace
  2026-11-03 11:50 [PATCH net v3 0/4] net/sched: fix short-skb metadata and namespace boundaries Jamal Hadi Salim
  2026-11-03 11:50 ` [PATCH net v3 1/4] net/sched: act_ife: encode configured zero metadata values Jamal Hadi Salim
  2026-11-03 11:50 ` [PATCH net v3 2/4] net/sched: act_ife: require a full Ethernet header on egress Jamal Hadi Salim
@ 2026-11-03 11:50 ` Jamal Hadi Salim
  2026-11-07 14:50   ` netdev-bot+sashiko
  2026-11-10 18:23 ` [PATCH net v3 0/4] net/sched: fix short-skb metadata and namespace boundaries patchwork-bot+netdevbpf
  3 siblings, 1 reply; 12+ messages in thread
From: Jamal Hadi Salim @ 2026-11-03 11:50 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/
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/SER-NSBD.v1.r2.20261031160050-3%40mojatatu.com
Link: https://lore.kernel.org/netdev/179366641863.488856.17368571591466000274@kernel.org/
Cc: stable@vger.kernel.org
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v1 -> v2: considers comments from Sashiko (nipa) on the repost2 posting:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/SER-NSBD.v1.r2.20261031160050-3%40mojatatu.com

The cross-netns check ran before the "teql can only be used as root"
check, so every case it did not catch was answered by that check, masking
the assertion that tc-testing case 124e makes.  The root-parent
validation now runs first and the namespace check second; 4/4 adds a
case that covers the namespace rejection on its own.

 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 70c948a26f80..dc21cd9704a2 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -200,6 +200,8 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
 	struct Qdisc *first;
 	unsigned long flags;
 
+	skb_queue_head_init(&q->q);
+
 	if (dev->hard_header_len > m->dev->hard_header_len)
 		return -EINVAL;
 
@@ -211,9 +213,12 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
 		return -EOPNOTSUPP;
 	}
 
-	q->m = m;
+	if (dev_net(dev) != dev_net(m->dev)) {
+		NL_SET_ERR_MSG_MOD(extack, "teql master is in a different network namespace");
+		return -EOPNOTSUPP;
+	}
 
-	skb_queue_head_init(&q->q);
+	q->m = m;
 
 	raw_spin_lock_irqsave(&m->slaves_lock, flags);
 	first = rcu_dereference_protected(m->slaves, lockdep_is_held(&m->slaves_lock));
-- 
2.43.0


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

* Re: [PATCH net v3 2/4] net/sched: act_ife: require a full Ethernet header on egress
  2026-11-03 11:50 ` [PATCH net v3 2/4] net/sched: act_ife: require a full Ethernet header on egress Jamal Hadi Salim
@ 2026-11-07 14:50   ` netdev-bot+sashiko
  2026-11-07 22:10     ` Jamal Hadi Salim
  0 siblings, 1 reply; 12+ messages in thread
From: netdev-bot+sashiko @ 2026-11-07 14:50 UTC (permalink / raw)
  To: jhs
  Cc: netdev, stable, shawnq1, davem, edumazet, horms, jiri, kuba,
	pabeni, victor, thomas.goff

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit is titled "require a full Ethernet header on
  egress".
- [Low] The commit message says "A short egress skb is now dropped
  instead of being encoded." The patch actually drops every egress skb…

--

net/sched: act_ife: require a full Ethernet header on egress

On egress, tcf_ife_is_eth_skb() now requires pskb_may_pull(skb, ETH_HLEN)
to succeed. Without that check, ife_encode() could copy ETH_HLEN bytes from
past the linear data of a short skb. The ingress mac_len check is unchanged.

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/

> diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
> index 6b23a3965121e..5a527da005661 100644
> --- a/net/sched/act_ife.c
> +++ b/net/sched/act_ife.c
> @@ -878,13 +878,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);

[Severity: Medium]
The subject says "require a full Ethernet header on egress". The commit
message also says tcf_ife_is_eth_skb() "returns true even when the skb has
no Ethernet header".

Does the new egress branch check that the skb has an Ethernet header, or
only that ETH_HLEN bytes are in the linear area?

Take the reproducer setup from the commit message and send an IP packet of
14 bytes or more instead of a short one:

tun_get_user() (IFF_TUN)
    skb_reset_network_header(skb);       /* mac_len is 0 */
tcf_mirred_to_dev()
    mac_len = at_ingress ? skb->mac_len : skb_network_offset(skb);
    skb_push_rcsum(skb_to_send, mac_len); /* pushes 0 bytes */
lo egress: tcf_ife_act()
    tcf_ife_is_eth_skb()
        pskb_may_pull(skb, ETH_HLEN)      /* succeeds */
    tcf_ife_encode()->ife_encode()
        iethh = (struct ethhdr *) skb->data;
        __skb_push(skb, total_push);
        memcpy(skb->data, iethh, ETH_HLEN);

At this point skb->data still points at the IP header. The first 14 IP
header bytes are copied as the inner "original" Ethernet header. They also
end up in the outer h_dest/h_source unless eth_dst/eth_src are configured.

The over-read past the linear data is fixed, and these bytes are packet
data the sender controls, not kernel memory. This behaviour for L3 skbs of
14 bytes or more was already there after commit 5b483f7791b0. Still, a
non-Ethernet frame can be encoded on egress as if it had an Ethernet
header, which is what the subject and issue statement say this patch
prevents.

The comment above the function also still reads as a pure check:

    IFE encapsulates the original Ethernet header and, on decode, expects
    to find one, so it can only ever work on skbs that carry one.

It does not say that the egress branch only checks linear length, or that
the call can now pull and so modify the skb.

Should the subject, issue statement and function comment say that egress
only checks linear length? Or should the egress branch also check that the
L2 header really is Ethernet?

[Severity: Low]
The commit message says "A short egress skb is now dropped instead of
being encoded." Is that the only behaviour change?

tcf_ife_act() calls tcf_ife_is_eth_skb() before choosing between encode
and decode. So a short egress skb is now dropped even when tcf_ife_encode()
would never have encoded it.

ife_get_sz() returns 0 when no metadata is present. One example is
ife_check_meta_u32() with no skb->mark and no configured value. In that
case tcf_ife_encode() returned before touching skb->data:

net/sched/act_ife.c:tcf_ife_encode() {
    ...
	if (!metalen) {		/* no metadata to send */
		...
		qstats_cpu_overlimit_inc(ife->common.cpu_qstats);
		return action;
	}
    ...
}

These sub-ETH_HLEN frames used to pass with the configured action and were
counted as overlimits. Now tcf_ife_act() drops them with TC_ACT_SHOT and
counts them as drops.

Could the commit message mention this change in behaviour and statistics?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/SER-NSBD.v3.r0.20261103064310%40mojatatu.com

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

* Re: [PATCH net v3 3/4] net/sched: sch_teql: reject qdiscs from another network namespace
  2026-11-03 11:50 ` [PATCH net v3 3/4] net/sched: sch_teql: reject qdiscs from another network namespace Jamal Hadi Salim
@ 2026-11-07 14:50   ` netdev-bot+sashiko
  2026-11-07 22:13     ` Jamal Hadi Salim
  0 siblings, 1 reply; 12+ messages in thread
From: netdev-bot+sashiko @ 2026-11-07 14:50 UTC (permalink / raw)
  To: jhs
  Cc: netdev, stable, shawnq1, davem, edumazet, horms, jiri, kuba,
	pabeni, victor, thomas.goff

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The fix checks that the slave device and the master are in the
  same network namespace only once, in teql_qdisc_init()…
- [Low] The commit message says only that a qdisc from another netns
  could attach to the init-net master "and change its MTU and flags".
- [Low] Moving skb_queue_head_init(&q->q) to the top of teql_qdisc_init()
  (sch_teql.c:203) is presented as groundwork for the new netns failure…

--

net/sched: sch_teql: reject qdiscs from another network namespace

This patch makes teql_qdisc_init() reject a teql qdisc whose device is
in a different network namespace than the teql master. It returns
-EOPNOTSUPP with an extack message. It also moves skb_queue_head_init()
of the private queue to the top of teql_qdisc_init(), so every
constructor failure path sees an initialized queue.

Link: https://lore.kernel.org/netdev/20261022144830.67948-1-shawnq1@163.com/
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/SER-NSBD.v1.r2.20261031160050-3%40mojatatu.com
Link: https://lore.kernel.org/netdev/179366641863.488856.17368571591466000274@kernel.org/

> 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.

[Severity: Low]
Does this description understate the impact of the bug?

Besides changing the MTU and flags, teql_qdisc_init() links the foreign
qdisc into the master's slave ring, or makes it the first slave:

    rcu_assign_pointer(NEXT_SLAVE(first), sch);

>From then on, teql_master_xmit() round-robins init_net traffic sent
through teqlN onto the device from the other netns. There is no netns
check and no skb scrubbing:

    skb->dev = slave;
    if (!netif_xmit_frozen_or_stopped(slave_txq))
        rc = netdev_start_xmit(skb, slave, slave_txq, false);

If teql0 is already up with slaves, the attach still succeeds as long as
the foreign device has compatible flags and an MTU at least as large as
the master's. A CAP_NET_ADMIN holder in a user namespace can set both.

With a veth slave, the netns owner could capture or drop part of the
host's teql traffic. It could also answer ARP that __teql_resolve()
sends on that device.

The code change closes this path as well. Since stable and CVE triage
depend on the stated impact, could the commit message describe the
cross-netns traffic interception?

> 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.

[Severity: Low]
This isn't a bug, but should the commit message say that this reorder
also fixes an existing NULL pointer dereference?

Before this patch, three returns happened before skb_queue_head_init():
-EINVAL (hard_header_len), -ELOOP (m->dev == dev) and -EOPNOTSUPP
(non-root parent). On those paths q->q was still zeroed. When init
fails, qdisc_create_dflt() calls qdisc_put(), which leads to:

    qdisc_put()
      __qdisc_destroy()
        qdisc_reset()
          teql_reset()
            skb_queue_purge(&dat->q)

skb_queue_purge_reason() sees list->next == NULL. That is not equal to
list, so it treats the queue as non-empty, and skb_queue_splice_init()
dereferences a NULL first pointer.

This looks reachable with net.core.default_qdisc=teql0. mq, mqprio and
taprio create children with a non-root parent, and
attach_one_default_qdisc() on teql0 itself returns -ELOOP. Setting
default_qdisc needs CAP_NET_ADMIN in init_net.

Would it be worth calling out the crash fix explicitly, for backports
and Fixes: attribution?

> diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
> index 70c948a26f809..dc21cd9704a25 100644
> --- a/net/sched/sch_teql.c
> +++ b/net/sched/sch_teql.c

[ ... ]

> @@ -211,9 +213,12 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
>  		return -EOPNOTSUPP;
>  	}
>  
> -	q->m = m;
> +	if (dev_net(dev) != dev_net(m->dev)) {
> +		NL_SET_ERR_MSG_MOD(extack, "teql master is in a different network namespace");
> +		return -EOPNOTSUPP;
> +	}

[Severity: Medium]
Is a netns check at attach time enough to keep the master and its
slaves in the same namespace?

teql_master_setup() never sets dev->netns_immutable. That means
__dev_change_net_namespace() lets teqlN move to another netns while
slaves are attached, because this is the only guard:

    if (dev->netns_immutable) {
        NL_SET_ERR_MSG(extack, "The interface netns is immutable");
        goto out;
    }

The move runs dev_shutdown() only on teqlN's own qdisc. The teql slave
qdiscs on the init-net devices stay linked in m->slaves:
teql_master_close() only stops the queue, and teql registers no netdev
notifier.

Once the master is in netns B, three things can happen:

The owner of B, who may be root in an unprivileged user namespace,
passes the new dev_net(dev) == dev_net(m->dev) check. It can then add
its own devices to a ring that still holds init-net slaves.

teql_master_open() takes the MTU and flags from the init-net slaves.

Traffic sent through teqlN in B reaches teql_master_xmit(), which sets
skb->dev = slave and calls netdev_start_xmit() on the init-net slave.
Through __teql_resolve(), it also calls ipv4_neigh_create() on the
init-net slave.

Moving teqlN out of init_net needs CAP_NET_ADMIN in init_net, so the
unprivileged user cannot do this alone. After this patch, though,
moving the master seems to be the only way a container can use teql.

Should the master be marked netns_immutable? Alternatively, should a
netns change of the master be refused, or detach the existing slaves?

> -	skb_queue_head_init(&q->q);
> +	q->m = m;
>  
>  	raw_spin_lock_irqsave(&m->slaves_lock, flags);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/SER-NSBD.v3.r0.20261103064310%40mojatatu.com

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

* Re: [PATCH net v3 2/4] net/sched: act_ife: require a full Ethernet header on egress
  2026-11-07 14:50   ` netdev-bot+sashiko
@ 2026-11-07 22:10     ` Jamal Hadi Salim
  2026-11-09 16:20       ` Simon Horman
  0 siblings, 1 reply; 12+ messages in thread
From: Jamal Hadi Salim @ 2026-11-07 22:10 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: netdev, stable, shawnq1, davem, edumazet, horms, jiri, kuba,
	pabeni, victor, thomas.goff

On Sat, Nov 7, 2026 at 9:50 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The commit is titled "require a full Ethernet header on
>   egress".

Yeah, that is a bit of an overclaim  - but i will not be resending
just for this. If there are code changes needed in the next patches i
will resend.

> - [Low] The commit message says "A short egress skb is now dropped
>   instead of being encoded." The patch actually drops every egress skb…

Same here. No resend coming.

cheers,
jamal
> --
>
> net/sched: act_ife: require a full Ethernet header on egress
>
> On egress, tcf_ife_is_eth_skb() now requires pskb_may_pull(skb, ETH_HLEN)
> to succeed. Without that check, ife_encode() could copy ETH_HLEN bytes from
> past the linear data of a short skb. The ingress mac_len check is unchanged.
>
> 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/
>
> > diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
> > index 6b23a3965121e..5a527da005661 100644
> > --- a/net/sched/act_ife.c
> > +++ b/net/sched/act_ife.c
> > @@ -878,13 +878,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);
>
> [Severity: Medium]
> The subject says "require a full Ethernet header on egress". The commit
> message also says tcf_ife_is_eth_skb() "returns true even when the skb has
> no Ethernet header".
>
> Does the new egress branch check that the skb has an Ethernet header, or
> only that ETH_HLEN bytes are in the linear area?
>
> Take the reproducer setup from the commit message and send an IP packet of
> 14 bytes or more instead of a short one:
>
> tun_get_user() (IFF_TUN)
>     skb_reset_network_header(skb);       /* mac_len is 0 */
> tcf_mirred_to_dev()
>     mac_len = at_ingress ? skb->mac_len : skb_network_offset(skb);
>     skb_push_rcsum(skb_to_send, mac_len); /* pushes 0 bytes */
> lo egress: tcf_ife_act()
>     tcf_ife_is_eth_skb()
>         pskb_may_pull(skb, ETH_HLEN)      /* succeeds */
>     tcf_ife_encode()->ife_encode()
>         iethh = (struct ethhdr *) skb->data;
>         __skb_push(skb, total_push);
>         memcpy(skb->data, iethh, ETH_HLEN);
>
> At this point skb->data still points at the IP header. The first 14 IP
> header bytes are copied as the inner "original" Ethernet header. They also
> end up in the outer h_dest/h_source unless eth_dst/eth_src are configured.
>
> The over-read past the linear data is fixed, and these bytes are packet
> data the sender controls, not kernel memory. This behaviour for L3 skbs of
> 14 bytes or more was already there after commit 5b483f7791b0. Still, a
> non-Ethernet frame can be encoded on egress as if it had an Ethernet
> header, which is what the subject and issue statement say this patch
> prevents.
>
> The comment above the function also still reads as a pure check:
>
>     IFE encapsulates the original Ethernet header and, on decode, expects
>     to find one, so it can only ever work on skbs that carry one.
>
> It does not say that the egress branch only checks linear length, or that
> the call can now pull and so modify the skb.
>
> Should the subject, issue statement and function comment say that egress
> only checks linear length? Or should the egress branch also check that the
> L2 header really is Ethernet?
>
> [Severity: Low]
> The commit message says "A short egress skb is now dropped instead of
> being encoded." Is that the only behaviour change?
>
> tcf_ife_act() calls tcf_ife_is_eth_skb() before choosing between encode
> and decode. So a short egress skb is now dropped even when tcf_ife_encode()
> would never have encoded it.
>
> ife_get_sz() returns 0 when no metadata is present. One example is
> ife_check_meta_u32() with no skb->mark and no configured value. In that
> case tcf_ife_encode() returned before touching skb->data:
>
> net/sched/act_ife.c:tcf_ife_encode() {
>     ...
>         if (!metalen) {         /* no metadata to send */
>                 ...
>                 qstats_cpu_overlimit_inc(ife->common.cpu_qstats);
>                 return action;
>         }
>     ...
> }
>
> These sub-ETH_HLEN frames used to pass with the configured action and were
> counted as overlimits. Now tcf_ife_act() drops them with TC_ACT_SHOT and
> counts them as drops.
>
> Could the commit message mention this change in behaviour and statistics?
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/SER-NSBD.v3.r0.20261103064310%40mojatatu.com

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

* Re: [PATCH net v3 3/4] net/sched: sch_teql: reject qdiscs from another network namespace
  2026-11-07 14:50   ` netdev-bot+sashiko
@ 2026-11-07 22:13     ` Jamal Hadi Salim
  2026-11-09 16:20       ` Simon Horman
  0 siblings, 1 reply; 12+ messages in thread
From: Jamal Hadi Salim @ 2026-11-07 22:13 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: netdev, stable, shawnq1, davem, edumazet, horms, jiri, kuba,
	pabeni, victor, thomas.goff

On Sat, Nov 7, 2026 at 9:50 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 2
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The fix checks that the slave device and the master are in the
>   same network namespace only once, in teql_qdisc_init()…
> - [Low] The commit message says only that a qdisc from another netns
>   could attach to the init-net master "and change its MTU and flags".
> - [Low] Moving skb_queue_head_init(&q->q) to the top of teql_qdisc_init()
>   (sch_teql.c:203) is presented as groundwork for the new netns failure…
>

Again, cosmetics i will fix if i have to resend.
The netns_immutable gap is pre-existing and needs CAP_NET_ADMIN in
init_net, so it stays the net-next follow-up.
The reorder keeps this patch's Fixes: because the namespace refusal is
itself reachable through qdisc_create_dflt() -> qdisc_reset() ->
teql_reset(), and if i have to resend then I will state the
interception impact in the body...


cheers,
jamal
> --
>
> net/sched: sch_teql: reject qdiscs from another network namespace
>
> This patch makes teql_qdisc_init() reject a teql qdisc whose device is
> in a different network namespace than the teql master. It returns
> -EOPNOTSUPP with an extack message. It also moves skb_queue_head_init()
> of the private queue to the top of teql_qdisc_init(), so every
> constructor failure path sees an initialized queue.
>
> Link: https://lore.kernel.org/netdev/20261022144830.67948-1-shawnq1@163.com/
> Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/SER-NSBD.v1.r2.20261031160050-3%40mojatatu.com
> Link: https://lore.kernel.org/netdev/179366641863.488856.17368571591466000274@kernel.org/
>
> > 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.
>
> [Severity: Low]
> Does this description understate the impact of the bug?
>
> Besides changing the MTU and flags, teql_qdisc_init() links the foreign
> qdisc into the master's slave ring, or makes it the first slave:
>
>     rcu_assign_pointer(NEXT_SLAVE(first), sch);
>
> >From then on, teql_master_xmit() round-robins init_net traffic sent
> through teqlN onto the device from the other netns. There is no netns
> check and no skb scrubbing:
>
>     skb->dev = slave;
>     if (!netif_xmit_frozen_or_stopped(slave_txq))
>         rc = netdev_start_xmit(skb, slave, slave_txq, false);
>
> If teql0 is already up with slaves, the attach still succeeds as long as
> the foreign device has compatible flags and an MTU at least as large as
> the master's. A CAP_NET_ADMIN holder in a user namespace can set both.
>
> With a veth slave, the netns owner could capture or drop part of the
> host's teql traffic. It could also answer ARP that __teql_resolve()
> sends on that device.
>
> The code change closes this path as well. Since stable and CVE triage
> depend on the stated impact, could the commit message describe the
> cross-netns traffic interception?
>
> > 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.
>
> [Severity: Low]
> This isn't a bug, but should the commit message say that this reorder
> also fixes an existing NULL pointer dereference?
>
> Before this patch, three returns happened before skb_queue_head_init():
> -EINVAL (hard_header_len), -ELOOP (m->dev == dev) and -EOPNOTSUPP
> (non-root parent). On those paths q->q was still zeroed. When init
> fails, qdisc_create_dflt() calls qdisc_put(), which leads to:
>
>     qdisc_put()
>       __qdisc_destroy()
>         qdisc_reset()
>           teql_reset()
>             skb_queue_purge(&dat->q)
>
> skb_queue_purge_reason() sees list->next == NULL. That is not equal to
> list, so it treats the queue as non-empty, and skb_queue_splice_init()
> dereferences a NULL first pointer.
>
> This looks reachable with net.core.default_qdisc=teql0. mq, mqprio and
> taprio create children with a non-root parent, and
> attach_one_default_qdisc() on teql0 itself returns -ELOOP. Setting
> default_qdisc needs CAP_NET_ADMIN in init_net.
>
> Would it be worth calling out the crash fix explicitly, for backports
> and Fixes: attribution?
>
> > diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
> > index 70c948a26f809..dc21cd9704a25 100644
> > --- a/net/sched/sch_teql.c
> > +++ b/net/sched/sch_teql.c
>
> [ ... ]
>
> > @@ -211,9 +213,12 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
> >               return -EOPNOTSUPP;
> >       }
> >
> > -     q->m = m;
> > +     if (dev_net(dev) != dev_net(m->dev)) {
> > +             NL_SET_ERR_MSG_MOD(extack, "teql master is in a different network namespace");
> > +             return -EOPNOTSUPP;
> > +     }
>
> [Severity: Medium]
> Is a netns check at attach time enough to keep the master and its
> slaves in the same namespace?
>
> teql_master_setup() never sets dev->netns_immutable. That means
> __dev_change_net_namespace() lets teqlN move to another netns while
> slaves are attached, because this is the only guard:
>
>     if (dev->netns_immutable) {
>         NL_SET_ERR_MSG(extack, "The interface netns is immutable");
>         goto out;
>     }
>
> The move runs dev_shutdown() only on teqlN's own qdisc. The teql slave
> qdiscs on the init-net devices stay linked in m->slaves:
> teql_master_close() only stops the queue, and teql registers no netdev
> notifier.
>
> Once the master is in netns B, three things can happen:
>
> The owner of B, who may be root in an unprivileged user namespace,
> passes the new dev_net(dev) == dev_net(m->dev) check. It can then add
> its own devices to a ring that still holds init-net slaves.
>
> teql_master_open() takes the MTU and flags from the init-net slaves.
>
> Traffic sent through teqlN in B reaches teql_master_xmit(), which sets
> skb->dev = slave and calls netdev_start_xmit() on the init-net slave.
> Through __teql_resolve(), it also calls ipv4_neigh_create() on the
> init-net slave.
>
> Moving teqlN out of init_net needs CAP_NET_ADMIN in init_net, so the
> unprivileged user cannot do this alone. After this patch, though,
> moving the master seems to be the only way a container can use teql.
>
> Should the master be marked netns_immutable? Alternatively, should a
> netns change of the master be refused, or detach the existing slaves?
>
> > -     skb_queue_head_init(&q->q);
> > +     q->m = m;
> >
> >       raw_spin_lock_irqsave(&m->slaves_lock, flags);
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/SER-NSBD.v3.r0.20261103064310%40mojatatu.com

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

* Re: [PATCH net v3 1/4] net/sched: act_ife: encode configured zero metadata values
  2026-11-03 11:50 ` [PATCH net v3 1/4] net/sched: act_ife: encode configured zero metadata values Jamal Hadi Salim
@ 2026-11-09 16:20   ` Simon Horman
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2026-11-09 16:20 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev, stable, shaos, David S . Miller, Eric Dumazet, Jiri Pirko,
	Jakub Kicinski, Paolo Abeni, Victor Nogueira, Tom Goff

On Tue, Nov 03, 2026 at 06:50:29AM -0500, Jamal Hadi Salim wrote:
> 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
> Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Reviewed-by: Simon Horman <horms@kernel.org>


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

* Re: [PATCH net v3 2/4] net/sched: act_ife: require a full Ethernet header on egress
  2026-11-07 22:10     ` Jamal Hadi Salim
@ 2026-11-09 16:20       ` Simon Horman
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2026-11-09 16:20 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev-bot+sashiko, netdev, stable, shawnq1, davem, edumazet,
	jiri, kuba, pabeni, victor, thomas.goff

On Sat, Nov 07, 2026 at 05:10:07PM -0500, Jamal Hadi Salim wrote:
> On Sat, Nov 7, 2026 at 9:50 AM <netdev-bot+sashiko@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 2 potential
> > issue(s) to consider.
> >
> > Critical: 0 · High: 0 · Medium: 1 · Low: 1
> >
> > If you believe none of the issues are legitimate, please reply to each
> > issue of Medium severity or higher and explain why the AI is wrong.
> > If any of them are legitimate and you plan to respin, please let patchwork
> > know by including "pw-bot: cr" as a separate line at the end of your reply
> > (one such reply per series is enough).
> >
> > - [Medium] The commit is titled "require a full Ethernet header on
> >   egress".
> 
> Yeah, that is a bit of an overclaim  - but i will not be resending
> just for this. If there are code changes needed in the next patches i
> will resend.
> 
> > - [Low] The commit message says "A short egress skb is now dropped
> >   instead of being encoded." The patch actually drops every egress skb…
> 
> Same here. No resend coming.

Reviewed-by: Simon Horman <horms@kernel.org>


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

* Re: [PATCH net v3 3/4] net/sched: sch_teql: reject qdiscs from another network namespace
  2026-11-07 22:13     ` Jamal Hadi Salim
@ 2026-11-09 16:20       ` Simon Horman
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2026-11-09 16:20 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev-bot+sashiko, netdev, stable, shawnq1, davem, edumazet,
	jiri, kuba, pabeni, victor, thomas.goff

On Sat, Nov 07, 2026 at 05:13:10PM -0500, Jamal Hadi Salim wrote:
> On Sat, Nov 7, 2026 at 9:50 AM <netdev-bot+sashiko@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 3 potential
> > issue(s) to consider.
> >
> > Critical: 0 · High: 0 · Medium: 1 · Low: 2
> >
> > If you believe none of the issues are legitimate, please reply to each
> > issue of Medium severity or higher and explain why the AI is wrong.
> > If any of them are legitimate and you plan to respin, please let patchwork
> > know by including "pw-bot: cr" as a separate line at the end of your reply
> > (one such reply per series is enough).
> >
> > - [Medium] The fix checks that the slave device and the master are in the
> >   same network namespace only once, in teql_qdisc_init()…
> > - [Low] The commit message says only that a qdisc from another netns
> >   could attach to the init-net master "and change its MTU and flags".
> > - [Low] Moving skb_queue_head_init(&q->q) to the top of teql_qdisc_init()
> >   (sch_teql.c:203) is presented as groundwork for the new netns failure…
> >
> 
> Again, cosmetics i will fix if i have to resend.
> The netns_immutable gap is pre-existing and needs CAP_NET_ADMIN in
> init_net, so it stays the net-next follow-up.
> The reorder keeps this patch's Fixes: because the namespace refusal is
> itself reachable through qdisc_create_dflt() -> qdisc_reset() ->
> teql_reset(), and if i have to resend then I will state the
> interception impact in the body...

Reviewed-by: Simon Horman <horms@kernel.org>


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

* Re: [PATCH net v3 0/4] net/sched: fix short-skb metadata and namespace boundaries
  2026-11-03 11:50 [PATCH net v3 0/4] net/sched: fix short-skb metadata and namespace boundaries Jamal Hadi Salim
                   ` (2 preceding siblings ...)
  2026-11-03 11:50 ` [PATCH net v3 3/4] net/sched: sch_teql: reject qdiscs from another network namespace Jamal Hadi Salim
@ 2026-11-10 18:23 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-11-10 18:23 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev, stable, shawnq1, davem, edumazet, horms, jiri, kuba,
	pabeni, victor, sashiko-bot, thomas.goff, shuah, yotam.gi,
	linux-kselftest

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  3 Nov 2026 06:50:28 -0500 you wrote:
> This series fixes three independent boundary problems in net/sched, and
> covers the third one with tc-testing cases.
> 
> 1/4 act_ife: encode a configured zero metadata value.  The u32 and u16
> presence callbacks reserve an 8-byte TLV for a configured zero value, but
> the encoders returned without writing it, leaking stale skb headroom bytes.
> 
> [...]

Here is the summary with links:
  - [net,v3,1/4] net/sched: act_ife: encode configured zero metadata values
    https://git.kernel.org/netdev/net/c/7d02c07c738b
  - [net,v3,2/4] net/sched: act_ife: require a full Ethernet header on egress
    https://git.kernel.org/netdev/net/c/11dbce090834
  - [net,v3,3/4] net/sched: sch_teql: reject qdiscs from another network namespace
    https://git.kernel.org/netdev/net/c/1ae84bdd736f
  - [net,v3,4/4] selftests: tc-testing: teql: cover each attach rejection on its own
    https://git.kernel.org/netdev/net/c/367a50c3233e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2026-11-10 18:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 11:50 [PATCH net v3 0/4] net/sched: fix short-skb metadata and namespace boundaries Jamal Hadi Salim
2026-11-03 11:50 ` [PATCH net v3 1/4] net/sched: act_ife: encode configured zero metadata values Jamal Hadi Salim
2026-11-09 16:20   ` Simon Horman
2026-11-03 11:50 ` [PATCH net v3 2/4] net/sched: act_ife: require a full Ethernet header on egress Jamal Hadi Salim
2026-11-07 14:50   ` netdev-bot+sashiko
2026-11-07 22:10     ` Jamal Hadi Salim
2026-11-09 16:20       ` Simon Horman
2026-11-03 11:50 ` [PATCH net v3 3/4] net/sched: sch_teql: reject qdiscs from another network namespace Jamal Hadi Salim
2026-11-07 14:50   ` netdev-bot+sashiko
2026-11-07 22:13     ` Jamal Hadi Salim
2026-11-09 16:20       ` Simon Horman
2026-11-10 18:23 ` [PATCH net v3 0/4] net/sched: fix short-skb metadata and namespace boundaries patchwork-bot+netdevbpf

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