Skip to contentSkip to conversation

net/sched: act_ife: ensure ETH_HLEN is pullable after decap

PATCH In review Ren Wei 3 participants 3 messages Jun 7 – Jun 8 1 message not in the archive

Subsystems (4)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Ren Wei

author
#1 lore raw

PATCH 1/1 net/sched: act_ife: ensure ETH_HLEN is pullable after decap

From: Yong Wang <[email protected]>

tcf_ife_decode() calls eth_type_trans() after ife_decode() removes the outer IFE header and metadata. However, ife_decode() only makes the outer IFE header area pullable before advancing skb->data.

As a result, the decapsulated inner Ethernet header may still be shorter than ETH_HLEN in the linear data area, or entirely reside in frags. eth_type_trans() then operates on an skb that does not satisfy its expected Ethernet header access requirements, which can trigger a BUG in the pull path.

Fix this by requiring ETH_HLEN to be pullable before calling eth_type_trans(). If that fails, drop the packet just like the other decode error paths in tcf_ife_decode().

Fixes ef6980b6becb ("introduce IFE action")
Assisted-by Codex:gpt-5.4
---
 net/sched/act_ife.c | 5 +++++
 1 file changed, 5 insertions(+)
net/sched/act_ife.c +5 -0
index d5e8a91bb4eb..e0075e3f6949 100644
@@ -759,6 +759,11 @@ static int tcf_ife_decode(struct sk_buff *skb, const struct tc_action *a,
 		return TC_ACT_SHOT;
 	}
 
+	if (!pskb_may_pull(skb, ETH_HLEN)) {
+		qstats_drop_inc(this_cpu_ptr(ife->common.cpu_qstats));
+		return TC_ACT_SHOT;
+	}
+
 	skb->protocol = eth_type_trans(skb, skb->dev);
 	skb_reset_network_header(skb);
 
Signature · 1 line
2.34.1
maintainer
#2 lore raw
Quoted 7 lines · Ren Wei

From: Yong Wang <[email protected]>

tcf_ife_decode() calls eth_type_trans() after ife_decode() removes the outer IFE header and metadata. However, ife_decode() only makes the outer IFE header area pullable before advancing skb->data.

Quoted 6 lines

As a result, the decapsulated inner Ethernet header may still be shorter than ETH_HLEN in the linear data area, or entirely reside in frags. eth_type_trans() then operates on an skb that does not satisfy its expected Ethernet header access requirements, which can trigger a BUG in the pull path.

From inspection this looks legit. I did not run your poc. Could you create a tdc test instead? You should be able to craft a scapy ethernet frame using nsplugin. Something like this:
Ether(type=0xED3E) / Raw(b'\\x00\\x02\\xaa') would suffice. That would be more helpful for future reference as well.

Quoted 20 lines

Fix this by requiring ETH_HLEN to be pullable before calling eth_type_trans(). If that fails, drop the packet just like the other decode error paths in tcf_ife_decode().

Fixes ef6980b6becb ("introduce IFE action")
Assisted-by Codex:gpt-5.4
---
 net/sched/act_ife.c | 5 +++++
 1 file changed, 5 insertions(+)
net/sched/act_ife.c +0 -0
index d5e8a91bb4eb..e0075e3f6949 100644

net/sched/act_ife.c static int tcf_ife_decode(struct sk_buff *skb, const struct tc_action *a, · line 768

Quoted 10 lines
net/sched/act_ife.c +5 -0
@@ -759,6 +759,11 @@ static int tcf_ife_decode(struct sk_buff *skb, const struct tc_action *a,
                return TC_ACT_SHOT;
        }
 
+       if (!pskb_may_pull(skb, ETH_HLEN)) {
+               qstats_drop_inc(this_cpu_ptr(ife->common.cpu_qstats));
+               return TC_ACT_SHOT;
+       }
+
        skb->protocol = eth_type_trans(skb, skb->dev);
        skb_reset_network_header(skb);

We already do a pull in ife_decode(), so why not just extend that to have ETH_HLEN?
Given that IFE is always encaped over ethernet frames it should be fine to assume ethernet header.
i.e something like:
- if (unlikely(!pskb_may_pull(skb, total_pull)))
+ if (unlikely(!pskb_may_pull(skb, total_pull + ETH_HLEN)))

cheers,
jamal

Quoted 4 lines
net/sched/act_ife.c +0 -1
--

2.34.1

#3 lore raw
Quoted 15 lines · Jamal Hadi Salim
Quoted 7 lines · Ren Wei

From: Yong Wang <[email protected]>

tcf_ife_decode() calls eth_type_trans() after ife_decode() removes the outer IFE header and metadata. However, ife_decode() only makes the outer IFE header area pullable before advancing skb->data.

Quoted 6 lines

As a result, the decapsulated inner Ethernet header may still be shorter than ETH_HLEN in the linear data area, or entirely reside in frags. eth_type_trans() then operates on an skb that does not satisfy its expected Ethernet header access requirements, which can trigger a BUG in the pull path.

Quoted 6 lines

[...]
From inspection this looks legit. I did not run your poc. Could you create a tdc test instead? You should be able to craft a scapy ethernet frame using nsplugin. Something like this:
Ether(type=0xED3E) / Raw(b'\\x00\\x02\\xaa') would suffice. That would be more helpful for future reference as well.

Can you try to run this tdc test?

cheers,
Victor

From 6b7ba3d553dfa8e76bdad8438f9585fda2b2fc35 Mon Sep 17 00:00:00 2001
From: Victor Nogueira <[email protected]>
Date: Mon, 8 Jun 2026 10:03:48 -0300
Subject: [PATCH net] selftests/tc-testing: Verify IFE can handle truncated
inner Ethernet header

Add a tdc test that exercises the act_ife decode path with a malformed IFE packet whose encapsulated inner Ethernet header is truncated.

The injected frame has a valid outer Ethernet header (ethertype 0xED3E) and a minimal IFE header (metalen 2, i.e. no metadata TLVs), but the payload that should hold the original frame is a single byte instead of a full Ethernet header. Once ife_decode() strips the outer header and the IFE metadata, fewer than ETH_HLEN bytes are left, which previously let eth_type_trans() read past the end of the linear data.

---
 .../tc-testing/tc-tests/actions/ife.json      | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +55 -0
index 808aef4afe22..ece7ec41bf99 100644
@@ -1378,5 +1378,60 @@
         "teardown": [
             "$TC actions flush action ife"
         ]
+    },
+    {
+        "id": "4e6b",
+        "name": "Decode IFE packet with truncated inner Ethernet header",
+        "category": [
+            "actions",
+            "ife"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            [
+                "$TC actions flush action ife",
+                0,
+                1,
+                255
+            ],
+            "$TC qdisc add dev $DEV1 clsact"
+        ],
+        "scapy": [
+            {
+                "iface": "$DEV0",
+                "count": 1,
+                "packet": "Ether(type=0xED3E) / Raw(b'\\x00\\x02\\xaa')"
+            }
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV1 ingress protocol all matchall action ife decode pipe index 10",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s -j actions get action ife index 10",
+        "matchJSON": [
+            {
+              "total acts": 0
+            },
+            {
+              "actions": [
+                  {
+                      "kind": "ife",
+                      "mode": "decode",
+                      "index": 10,
+                      "stats": {
+                          "bytes": 3,
+                          "packets": 1,
+                          "drops": 1
+                      }
+                  }
+              ]
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DEV1 clsact"
+        ]
     }
 ]
Signature · 1 line
2.54.0