* [PATCH net v2 1/2] net/sched: act_skbmod: require a full Ethernet header for MAC rewrites
2026-07-23 17:54 [PATCH net v2 0/2] net/sched: act_vlan: pop_eth can strip non-Ethernet skbs and panic loopback Ren Wei
@ 2026-07-23 17:54 ` Ren Wei
2026-07-23 17:54 ` [PATCH net v2 2/2] net/sched: act_vlan: skip pop_eth on non-Ethernet devices Ren Wei
2026-07-25 10:46 ` [PATCH net v2 0/2] net/sched: act_vlan: pop_eth can strip non-Ethernet skbs and panic loopback Jamal Hadi Salim
2 siblings, 0 replies; 6+ messages in thread
From: Ren Wei @ 2026-07-23 17:54 UTC (permalink / raw)
To: netdev
Cc: jhs, jiri, davem, edumazet, pabeni, horms, peilin.ye, cong.wang,
gnault, vega, bronzed_45_vested, enjou1224z
From: Wyatt Feng <bronzed_45_vested@icloud.com>
tcf_skbmod_act() rewrites DMAC, SMAC, ETYPE and SWAPMAC through
eth_hdr(skb), but it derives the writable length from
skb_mac_header_len(skb). After a prior action strips or shortens the
L2 header, skbmod can still attempt the Ethernet rewrite without a
full Ethernet header being present.
Reject non-ECN skbmod rewrites unless the skb is on an Ethernet
device, the MAC header is set, and the MAC header length is at least
ETH_HLEN. Also use ETH_HLEN as the writable length for these rewrites,
which matches the bytes actually modified.
This fixes a buffer-size length miscalculation that can corrupt
short or non-linear packets.
Fixes: 56af5e749f20 ("net/sched: act_skbmod: Add SKBMOD_F_ECN option support")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:GPT-5.4
Signed-off-by: Wyatt Feng <bronzed_45_vested@icloud.com>
Signed-off-by: Ren Wei <enjou1224z@gmail.com>
---
net/sched/act_skbmod.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index a464b0a3c1b8..30c2eac17aff 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -38,7 +38,6 @@ TC_INDIRECT_SCOPE int tcf_skbmod_act(struct sk_buff *skb,
if (unlikely(p->action == TC_ACT_SHOT))
goto drop;
- max_edit_len = skb_mac_header_len(skb);
flags = p->flags;
/* tcf_skbmod_init() guarantees "flags" to be one of the following:
@@ -49,6 +48,8 @@ TC_INDIRECT_SCOPE int tcf_skbmod_act(struct sk_buff *skb,
* packets.
*/
if (flags == SKBMOD_F_ECN) {
+ max_edit_len = skb_mac_header_len(skb);
+
switch (skb_protocol(skb, true)) {
case cpu_to_be16(ETH_P_IP):
case cpu_to_be16(ETH_P_IPV6):
@@ -57,8 +58,12 @@ TC_INDIRECT_SCOPE int tcf_skbmod_act(struct sk_buff *skb,
default:
goto out;
}
- } else if (!skb->dev || skb->dev->type != ARPHRD_ETHER) {
+ } else if (!skb->dev || skb->dev->type != ARPHRD_ETHER ||
+ !skb_mac_header_was_set(skb) ||
+ skb_mac_header_len(skb) < ETH_HLEN) {
goto out;
+ } else {
+ max_edit_len = ETH_HLEN;
}
err = skb_ensure_writable(skb, max_edit_len);
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net v2 2/2] net/sched: act_vlan: skip pop_eth on non-Ethernet devices
2026-07-23 17:54 [PATCH net v2 0/2] net/sched: act_vlan: pop_eth can strip non-Ethernet skbs and panic loopback Ren Wei
2026-07-23 17:54 ` [PATCH net v2 1/2] net/sched: act_skbmod: require a full Ethernet header for MAC rewrites Ren Wei
@ 2026-07-23 17:54 ` Ren Wei
2026-07-25 10:46 ` [PATCH net v2 0/2] net/sched: act_vlan: pop_eth can strip non-Ethernet skbs and panic loopback Jamal Hadi Salim
2 siblings, 0 replies; 6+ messages in thread
From: Ren Wei @ 2026-07-23 17:54 UTC (permalink / raw)
To: netdev
Cc: jhs, jiri, davem, edumazet, pabeni, horms, peilin.ye, cong.wang,
gnault, vega, bronzed_45_vested, enjou1224z
From: Wyatt Feng <bronzed_45_vested@icloud.com>
tcf_vlan_act() currently applies TCA_VLAN_ACT_POP_ETH on any device
type. On loopback and other non-Ethernet devices, this can strip
ETH_HLEN bytes that later paths still expect to be an Ethernet header.
Treat POP_ETH as an Ethernet-only action and leave non-Ethernet
packets unchanged.
This fixes a short-packet handling bug that can trigger a BUG in
__skb_pull().
Fixes: 19fbcb36a39e ("net/sched: act_vlan: Add {POP,PUSH}_ETH actions")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:GPT-5.4
Signed-off-by: Wyatt Feng <bronzed_45_vested@icloud.com>
Signed-off-by: Ren Wei <enjou1224z@gmail.com>
---
net/sched/act_vlan.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 6a375fdc63dd..b77f649ae204 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -4,6 +4,7 @@
*/
#include <linux/module.h>
+#include <linux/if_arp.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/skbuff.h>
@@ -76,6 +77,9 @@ TC_INDIRECT_SCOPE int tcf_vlan_act(struct sk_buff *skb,
__vlan_hwaccel_put_tag(skb, p->tcfv_push_proto, tci);
break;
case TCA_VLAN_ACT_POP_ETH:
+ if (!skb->dev || skb->dev->type != ARPHRD_ETHER)
+ goto out;
+
err = skb_eth_pop(skb);
if (err)
goto drop;
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net v2 0/2] net/sched: act_vlan: pop_eth can strip non-Ethernet skbs and panic loopback
2026-07-23 17:54 [PATCH net v2 0/2] net/sched: act_vlan: pop_eth can strip non-Ethernet skbs and panic loopback Ren Wei
2026-07-23 17:54 ` [PATCH net v2 1/2] net/sched: act_skbmod: require a full Ethernet header for MAC rewrites Ren Wei
2026-07-23 17:54 ` [PATCH net v2 2/2] net/sched: act_vlan: skip pop_eth on non-Ethernet devices Ren Wei
@ 2026-07-25 10:46 ` Jamal Hadi Salim
2026-07-25 12:21 ` Jamal Hadi Salim
2 siblings, 1 reply; 6+ messages in thread
From: Jamal Hadi Salim @ 2026-07-25 10:46 UTC (permalink / raw)
To: Ren Wei
Cc: netdev, jiri, davem, edumazet, pabeni, horms, gnault, vega,
bronzed_45_vested
[-- Attachment #1: Type: text/plain, Size: 1292 bytes --]
On Thu, Jul 23, 2026 at 1:54 PM Ren Wei <enjou1224z@gmail.com> wrote:
>
> From: Wyatt Feng <bronzed_45_vested@icloud.com>
>
> Hi Linux kernel maintainers,
>
> We found an issue in net/sched/act_vlan.c.
> The bug is reachable by an unprivileged user using private user and network namespaces.
> The relevant details are provided below.
>
Please remove bouncing @bytedance emails from the CC list.
You did not address the discussion we had earlier, which is a very
good example of an issue Yuan brought up at the bof - "fixing the root
cause".
Attaching the last email from Guillaume since this was not on a public list.
I had argued you only need patch 2 (and to be safe handle both pop and
push) and Guillaume argued that would not fix the root cause and there
is potential the bug could still happen and patch 1 would be needed.
His exact wording is:
"
So, if patch 1 isn't needed, that means we must guarantee all skbs
reaching an ARPHRD_ETHER device to have at least ETH_HLEN bytes of
headroom.
"
after some back and forth we steered into drivers and pskb_may_pull()
invocation.
In any case it would be a good idea for you to review and consider the
discussion and infact test with suggested vrf device (which doesnt
pskb_may_pull())
cheers,
jamal
[-- Attachment #2: vlan.email --]
[-- Type: application/octet-stream, Size: 4996 bytes --]
Date: Fri, 3 Jul 2026 17:54:17 +0200
From: Guillaume Nault <gnault@redhat.com>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Ren Wei <n05ec@lzu.edu.cn>, jiri@resnulli.us, davem@davemloft.net,
edumazet@google.com, pabeni@redhat.com, horms@kernel.org,
cong.wang@bytedance.com, peilin.ye@bytedance.com,
yuantan098@gmail.com, yifanwucs@gmail.com, tomapufckgml@gmail.com,
zcliangcn@gmail.com, bird@lzu.edu.cn, bronzed_45_vested@icloud.com
Subject: Re: [PATCH net 0/2] net/sched: act_vlan: pop_eth can strip
non-Ethernet skbs and panic loopback
Message-ID: <akfbKaGBRiKG3P33@debian>
On Wed, Jul 01, 2026 at 12:13:53PM -0400, Jamal Hadi Salim wrote:
> Hi Guillaume,
>
> On Tue, Jun 30, 2026 at 10:28 AM Guillaume Nault <gnault@redhat.com> wrote:
> >
> > On Mon, Jun 29, 2026 at 06:31:46PM -0400, Jamal Hadi Salim wrote:
> > > On Sun, Jun 28, 2026 at 10:46 PM Ren Wei <n05ec@lzu.edu.cn> wrote:
> > > >
> > > > From: Wyatt Feng <bronzed_45_vested@icloud.com>
> > > >
> > > > Hi Linux kernel maintainers,
> > > >
> > > > We found an issue in net/sched/act_vlan.c.
> > > > The bug is reachable by an unprivileged user using private user and network namespaces.
> > > > The relevant details are provided below.
> > > >
> > > > ---- details below ----
> > > >
> > > > Bug details:
> > > >
> > > > The bug is in `tcf_vlan_act()` when handling `TCA_VLAN_ACT_POP_ETH`.
> > > > The action is applied without checking that the skb is on an Ethernet
> > > > device. On loopback and other non-Ethernet devices, `skb_eth_pop()`
> > > > removes `ETH_HLEN` bytes from the skb head even though the later TX path
> > > > still expects an Ethernet header.
> > > >
> > >
> > > I dont think you need patch 1.
> > > skbmod already checks at act_skbmod.c:60 a no-op on non-Ethernet
> > > devices like lo:
> > > ..
> > > } else if (!skb->dev || skb->dev->type != ARPHRD_ETHER) {
> > > goto out;
> > > }
> > > ..
> >
> > Looks like patch 1 might fix a real problem, although not the one
> > described in the cover letter.
> >
> > The non-SKBMOD_F_ECN cases require up to ETH_HLEN bytes of writable
> > headroom. However the skb_ensure_writable(skb, max_edit_len) call
> > currently doesn't give any such guarantee (max_edit_len is set with
> > skb_mac_header_len(skb), which might return 0 as shown in the
> > reproducer).
> >
> > So, if patch 1 isn't needed, that means we must guarantee all skbs
> > reaching an ARPHRD_ETHER device to have at least ETH_HLEN bytes of
> > headroom.
> >
>
> I re-run the poc with veth which ARPHRD_ETHER and nothing happened -
> certainly no report from KASAN.
> Something like:
>
> ip link add veth0 type veth peer name veth1
> ip link set veth0 mtu 16334 (or 65535)
> tc filter add dev veth0 egress ... action vlan pop_eth action skbmod dmac ...
>
> I think though (need to look closer) your contention that
> skb_ensure_writable is correct. I dont have time to prove it right now
> but likely it doesnt fail because may be overwritten by pskb_may_pull
> in veth_xmit(), so it works out just fine?
Yes, veth ensures it has enough headroom, so the problem cannot happen
with this driver.
> > > Patch 2 seems to fix the root cause, doesn't it? But even that could
> > > be simple example, not compile or tested as attached..
> >
> > It just prevents the problem from happening on loopback and non-Ethernet
> > devices. But what about all the ARPHRD_ETHER drivers?
> >
> > If we consider that the stack must guarantee skbs to have ETH_HLEN bytes
> > of headroom on ARPHRD_ETHER devices, then patch 2 doesn't fix the
> > problem (we can just change the reproducer and replace "lo" with an
> > Ethernet device that assumes ETH_HLEN of headroom).
> >
> > If, on the other hand, we consider that no assumption can be made on
> > skb's headroom, then we need to check that at the beginning of
> > loopback_xmit() (and potentially other drivers).
>
> See my comment on veth above. Comment on what you are saying here:
> Doing a grep on callers of eth_type_trans who use it on tx path shows
> only loopback and vrf dont do a pskb_may_pull() before invoking it.
> vrf is ARPHDR_ETHER, so what you are describing might be possible with
> VRF (I would consider that a VRF bug though).
> Perhaps the OP can test with VRF as well..
I also think it'd be safer for vrf to verify the headroom of its skbs,
if it doesn't already. Other virtual drivers generally do that. I'm not
so sure about physical drivers. If they don't, then limitting
TCA_VLAN_ACT_POP_ETH to ARPHRD_ETHER devices wouldn't fix the problem.
And even if we added such limitation, I don't see what would
prevent a user from configuring TCA_VLAN_ACT_POP_ETH on an Ethernet
device and redirect the resulting skb with no headroom to the loopback.
By the way, I'm going to be offline for a bit more than a week. So
sorry in advance for the delay in my future responses.
> cheers,
> jamal
>
^ permalink raw reply [flat|nested] 6+ messages in thread