* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox