[PATCH net] vlan: require the MAC header to be present in __vlan_insert_inner_tag()

[PATCH net] vlan: require the MAC header to be present in __vlan_insert_inner_tag()

[Xiang Mei]

__vlan_insert_inner_tag() only guarantees head room via skb_cow_head(),
never that mac_len bytes of MAC header are present.  Its ETH_HLEN
wrappers - __vlan_insert_tag() under skb_vlan_push(), and
vlan_insert_tag() under validate_xmit_vlan() on the generic transmit
path - therefore rewrite the first 16 bytes at skb->data: a 12-byte
memmove plus two 2-byte stores at +12 and +14.  No caller supplies the
bound, while the pop helpers use skb_ensure_writable()/pskb_may_pull().

An IFF_TUN device has hard_header_len == 0, so packet_snd() accepts a
one-byte AF_PACKET/SOCK_RAW frame.  The first vlan push only sets a
hwaccel tag; the next - clsact "action vlan push" or
bpf_skb_vlan_push() - enters the helper with skb->len still 1.  The
head comes from skbuff_small_head without __GFP_ZERO, so each push
drags bytes from beyond skb->tail into the frame.  After three the
one-byte send leaves as 13 bytes carrying 11 bytes of uninitialised
slab:

  0000: 5a b3 62 12 80 88 ff ff 00 b3 62 12 81
           `------------------------------'
  only 0x5a was sent; the rest is slab, here the top 56 bits of a
  linear-map address

Require the MAC header the helper rewrites to be present, so such a
frame is dropped rather than transmitted.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reported-by: co+0ea1ac045375cf05@bugs.sh
Assisted-by: LLM
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
 include/linux/if_vlan.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 20cc16ea4e5a..4846032bf4ff 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -365,6 +365,9 @@ static inline int __vlan_insert_inner_tag(struct sk_buff *skb,
 	const u8 meta_len = mac_len > ETH_TLEN ? skb_metadata_len(skb) : 0;
 	struct vlan_ethhdr *veth;
 
+	if (unlikely(!pskb_may_pull(skb, mac_len)))
+		return -EINVAL;
+
 	if (skb_cow_head(skb, meta_len + VLAN_HLEN) < 0)
 		return -ENOMEM;
 
-- 
2.43.0

Re: [PATCH net] vlan: require the MAC header to be present in __vlan_insert_inner_tag()

[Simon Horman]

On Tue, Sep 15, 2026 at 01:31:52AM -0700, Xiang Mei wrote:
> __vlan_insert_inner_tag() only guarantees head room via skb_cow_head(),
> never that mac_len bytes of MAC header are present.  Its ETH_HLEN
> wrappers - __vlan_insert_tag() under skb_vlan_push(), and
> vlan_insert_tag() under validate_xmit_vlan() on the generic transmit
> path - therefore rewrite the first 16 bytes at skb->data: a 12-byte
> memmove plus two 2-byte stores at +12 and +14.  No caller supplies the
> bound, while the pop helpers use skb_ensure_writable()/pskb_may_pull().
> 
> An IFF_TUN device has hard_header_len == 0, so packet_snd() accepts a
> one-byte AF_PACKET/SOCK_RAW frame.  The first vlan push only sets a
> hwaccel tag; the next - clsact "action vlan push" or
> bpf_skb_vlan_push() - enters the helper with skb->len still 1.  The
> head comes from skbuff_small_head without __GFP_ZERO, so each push
> drags bytes from beyond skb->tail into the frame.  After three the
> one-byte send leaves as 13 bytes carrying 11 bytes of uninitialised
> slab:
> 
>   0000: 5a b3 62 12 80 88 ff ff 00 b3 62 12 81
>            `------------------------------'
>   only 0x5a was sent; the rest is slab, here the top 56 bits of a
>   linear-map address
> 
> Require the MAC header the helper rewrites to be present, so such a
> frame is dropped rather than transmitted.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Reported-by: co+0ea1ac045375cf05@bugs.sh
> Assisted-by: LLM
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> ---
>  include/linux/if_vlan.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index 20cc16ea4e5a..4846032bf4ff 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -365,6 +365,9 @@ static inline int __vlan_insert_inner_tag(struct sk_buff *skb,
>  	const u8 meta_len = mac_len > ETH_TLEN ? skb_metadata_len(skb) : 0;
>  	struct vlan_ethhdr *veth;
>  
> +	if (unlikely(!pskb_may_pull(skb, mac_len)))
> +		return -EINVAL;
> +
>  	if (skb_cow_head(skb, meta_len + VLAN_HLEN) < 0)
>  		return -ENOMEM;

TBH I am surprised that we have a bug like this in this function.
But your analysis matches my understanding of the code.

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

Re: [PATCH net] vlan: require the MAC header to be present in __vlan_insert_inner_tag()

[Jakub Kicinski]

On Sat, 19 Sep 2026 08:45:35 +0100 Simon Horman wrote:
> > +	if (unlikely(!pskb_may_pull(skb, mac_len)))
> > +		return -EINVAL;
> > +
> >  	if (skb_cow_head(skb, meta_len + VLAN_HLEN) < 0)
> >  		return -ENOMEM;  
> 
> TBH I am surprised that we have a bug like this in this function.
> But your analysis matches my understanding of the code.

FWIW I suspect it's because non-ethernet packets shouldn't normally
reach ethernet helpers. But I'm not sure how practical it would be
to push mac_len checks to the callers or anything of the sort,
so we can as well check here..

Re: [PATCH net] vlan: require the MAC header to be present in __vlan_insert_inner_tag()

[Simon Horman]

On Sat, Sep 19, 2026 at 03:49:11PM -0700, Jakub Kicinski wrote:
> On Sat, 19 Sep 2026 08:45:35 +0100 Simon Horman wrote:
> > > +	if (unlikely(!pskb_may_pull(skb, mac_len)))
> > > +		return -EINVAL;
> > > +
> > >  	if (skb_cow_head(skb, meta_len + VLAN_HLEN) < 0)
> > >  		return -ENOMEM;  
> > 
> > TBH I am surprised that we have a bug like this in this function.
> > But your analysis matches my understanding of the code.
> 
> FWIW I suspect it's because non-ethernet packets shouldn't normally
> reach ethernet helpers.

Yes, I agree that seems likely.

> But I'm not sure how practical it would be
> to push mac_len checks to the callers or anything of the sort,
> so we can as well check here..

Re: [PATCH net] vlan: require the MAC header to be present in __vlan_insert_inner_tag()

[patchwork-bot+netdevbpf]

Hello:

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

On Tue, 15 Sep 2026 01:31:52 -0700 you wrote:
> __vlan_insert_inner_tag() only guarantees head room via skb_cow_head(),
> never that mac_len bytes of MAC header are present.  Its ETH_HLEN
> wrappers - __vlan_insert_tag() under skb_vlan_push(), and
> vlan_insert_tag() under validate_xmit_vlan() on the generic transmit
> path - therefore rewrite the first 16 bytes at skb->data: a 12-byte
> memmove plus two 2-byte stores at +12 and +14.  No caller supplies the
> bound, while the pop helpers use skb_ensure_writable()/pskb_may_pull().
> 
> [...]

Here is the summary with links:
  - [net] vlan: require the MAC header to be present in __vlan_insert_inner_tag()
    https://git.kernel.org/netdev/net/c/ab888242fce4

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