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

[Simon Horman <horms@kernel.org>]

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>