[PATCH net] net: limit priority VLAN untag depth

[PATCH net] net: limit priority VLAN untag depth

[Kamil Sawicki]

The priority VLAN receive path can repeatedly remove VID 0 tags and retry
packet processing. Unlike the read-only VLAN protocol parser, this path has
no limit on the number of nested tags it consumes.

Each skb_vlan_untag() advances skb->data and resets header offsets. The
offsets are stored in 16-bit fields. With enough nested tags, the network
or transport offset can wrap. A later protocol handler can then derive a
large pull length from the wrapped offsets and access memory outside the
skb allocation.

Apply the existing VLAN_MAX_DEPTH limit to the exceptional VID 0 retry.
Count the tag already stored in skb VLAN metadata as the first level, then
stop before untagging a ninth level. Keep the accounting inside this
uncommon branch so the regular VLAN receive path is unaffected.

Fixes: 36b2f61a42c2 ("net: handle 802.1P vlan 0 packets properly")
Cc: stable@vger.kernel.org
Assisted-by: openai:gpt-5.6-sol
Signed-off-by: Kamil Sawicki <user148783812897@gmail.com>
---
Tested on net/main f4377bf85887 (7.3-rc7) with KASAN. The patched kernel
blocked the nested vhost/TAP trigger after eight untag calls on five fresh
boots, with no tcp_v4_rcv call. The eight/nine-tag boundary, nonlinear
GSO, 32 MiB BIG TCP and fragmented-head tests passed.

An unpatched protocol-253 control executed 31 calls to
skb_copy_and_csum_bits() with offset 65516 and length 548. A direct shadow
probe recorded a 20-byte wrapped linear source outside the skb allocation;
all four Generic-KASAN shadow bytes were zero. The patched control completed
the same workload with zero matching sink calls and zero shadow-probe hits.

Allyesconfig and allmodconfig W=1 builds completed with CONFIG_WERROR=
because unrelated existing warnings stopped the default full builds. Both
produced vmlinux and bzImage, and net/core/dev.c emitted no warnings.

 net/core/dev.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index e76762e29360ea..b40efea27dc292 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6198,6 +6198,8 @@ static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
 	}
 
 	if (unlikely(skb_vlan_tag_present(skb)) && !netdev_uses_dsa(skb->dev)) {
+		unsigned int vlan_depth = 1;
+
 check_vlan_id:
 		if (skb_vlan_tag_get_id(skb)) {
 			/* Vlan id is non 0 and vlan_do_receive() above couldn't
@@ -6209,6 +6211,10 @@ static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
 			 * 802.1Q or 802.1AD and vlan_do_receive() above could
 			 * not find vlan dev for vlan id 0.
 			 */
+			if (unlikely(vlan_depth >= VLAN_MAX_DEPTH))
+				goto drop;
+
+			vlan_depth++;
 			__vlan_hwaccel_clear_tag(skb);
 			skb = skb_vlan_untag(skb);
 			if (unlikely(!skb))

base-commit: f4377bf85887f57b59abb5479f7660a81ffe4449
-- 
2.53.0

Re: [PATCH net] net: limit priority VLAN untag depth

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net: limit priority VLAN untag depth

[Kamil Sawicki]

Hi,

I discovered this issue while researching the Linux network receive path for
Google kvmCTF, with OpenAI Codex (gpt-5.6-sol) assisting the code analysis.

I manually reviewed the affected path, reproduced the issue through
vhost-net/TAP on Linux 6.1.74 and current net/main, and validated the fix with
differential KASAN, ftrace, boundary, GSO and BIG TCP tests. The existing
Assisted-by tag records the model used.

Thanks,
Kamil

Re: [PATCH net] net: limit priority VLAN untag depth

[Willem de Bruijn]

Kamil Sawicki wrote:
> The priority VLAN receive path can repeatedly remove VID 0 tags and retry
> packet processing. Unlike the read-only VLAN protocol parser, this path has
> no limit on the number of nested tags it consumes.
> 
> Each skb_vlan_untag() advances skb->data and resets header offsets. The
> offsets are stored in 16-bit fields. With enough nested tags, the network
> or transport offset can wrap. A later protocol handler can then derive a
> large pull length from the wrapped offsets and access memory outside the
> skb allocation.
> 
> Apply the existing VLAN_MAX_DEPTH limit to the exceptional VID 0 retry.
> Count the tag already stored in skb VLAN metadata as the first level, then
> stop before untagging a ninth level. Keep the accounting inside this
> uncommon branch so the regular VLAN receive path is unaffected.
> 
> Fixes: 36b2f61a42c2 ("net: handle 802.1P vlan 0 packets properly")

Thanks for the patch. Agreed on the general approach.

One small suggestion for the drop_reason is worth a respin imho.
But aside from that

Reviewed-by: Willem de Bruijn <willemb@google.com>

Just curious: is more than one 802.1P vlan 0 packet preceding a real
vlan tag even something that would occur in practice?

> Cc: stable@vger.kernel.org
> Assisted-by: openai:gpt-5.6-sol
> Signed-off-by: Kamil Sawicki <user148783812897@gmail.com>
> ---
> Tested on net/main f4377bf85887 (7.3-rc7) with KASAN. The patched kernel
> blocked the nested vhost/TAP trigger after eight untag calls on five fresh
> boots, with no tcp_v4_rcv call. The eight/nine-tag boundary, nonlinear
> GSO, 32 MiB BIG TCP and fragmented-head tests passed.
> 
> An unpatched protocol-253 control executed 31 calls to
> skb_copy_and_csum_bits() with offset 65516 and length 548. A direct shadow
> probe recorded a 20-byte wrapped linear source outside the skb allocation;
> all four Generic-KASAN shadow bytes were zero. The patched control completed
> the same workload with zero matching sink calls and zero shadow-probe hits.
> 
> Allyesconfig and allmodconfig W=1 builds completed with CONFIG_WERROR=
> because unrelated existing warnings stopped the default full builds. Both
> produced vmlinux and bzImage, and net/core/dev.c emitted no warnings.
> 
>  net/core/dev.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index e76762e29360ea..b40efea27dc292 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6198,6 +6198,8 @@ static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
>  	}
>  
>  	if (unlikely(skb_vlan_tag_present(skb)) && !netdev_uses_dsa(skb->dev)) {
> +		unsigned int vlan_depth = 1;
> +

nit: since this does not count the actual vlan tags removed with jumps
to another_round, slightly confusing naming. Maybe 8021p_depth
or so.

>  check_vlan_id:
>  		if (skb_vlan_tag_get_id(skb)) {
>  			/* Vlan id is non 0 and vlan_do_receive() above couldn't
> @@ -6209,6 +6211,10 @@ static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
>  			 * 802.1Q or 802.1AD and vlan_do_receive() above could
>  			 * not find vlan dev for vlan id 0.
>  			 */
> +			if (unlikely(vlan_depth >= VLAN_MAX_DEPTH))
> +				goto drop;

consider

        if (unlikely(vlan_depth >= VLAN_MAX_DEPTH)) {
            drop_reason = SKB_DROP_REASON_RECURSION_LIMIT;
            goto drop;
        }

> +
> +			vlan_depth++;
>  			__vlan_hwaccel_clear_tag(skb);
>  			skb = skb_vlan_untag(skb);
>  			if (unlikely(!skb))
> 
> base-commit: f4377bf85887f57b59abb5479f7660a81ffe4449
> -- 
> 2.53.0
> 

Re: [PATCH net] net: limit priority VLAN untag depth

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The commit message promises a per-packet bound, but the patch
  only bounds one pass through __netif_receive_skb_core()…

--

net: limit priority VLAN untag depth

This patch adds a vlan_depth counter to the VID 0 retry branch in
__netif_receive_skb_core(). Once VLAN_MAX_DEPTH tags have been consumed,
the packet is dropped. The aim is to stop repeated skb_vlan_untag() calls
from wrapping the 16-bit network and transport header offsets.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index e76762e29360e..b40efea27dc29 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6198,6 +6198,8 @@ static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
>  	}
>  
>  	if (unlikely(skb_vlan_tag_present(skb)) && !netdev_uses_dsa(skb->dev)) {
> +		unsigned int vlan_depth = 1;
> +
>  check_vlan_id:
>  		if (skb_vlan_tag_get_id(skb)) {
>  			/* Vlan id is non 0 and vlan_do_receive() above couldn't
> @@ -6209,6 +6211,10 @@ static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
>  			 * 802.1Q or 802.1AD and vlan_do_receive() above could
>  			 * not find vlan dev for vlan id 0.
>  			 */
> +			if (unlikely(vlan_depth >= VLAN_MAX_DEPTH))
> +				goto drop;
> +
> +			vlan_depth++;
>  			__vlan_hwaccel_clear_tag(skb);
>  			skb = skb_vlan_untag(skb);
>  			if (unlikely(!skb))

[Severity: High]

Does this limit the number of tags removed per packet, or only per pass
through __netif_receive_skb_core()?

vlan_depth is declared inside the if block, so it goes back to 1 every
time processing jumps to another_round. The VID 0 branch itself leaves
through another_round when the inner tag matches a VLAN device:

			if (vlan_do_receive(&skb))
				/* After stripping off 802.1P header with vlan 0
				 * vlan dev is found for inner header.
				 */
				goto another_round;

A TC ingress peer redirect also restarts the round on the same skb, and
skb->data does not change:

net/core/dev.c:sch_handle_ingress() {
    ...
	__skb_push(skb, skb->mac_len);
	if (skb_do_redirect(skb) == -EAGAIN) {
		__skb_pull(skb, skb->mac_len);
		*another = true;
		break;
	}
    ...
}

With BPF_F_PEER, skb_do_redirect() calls skb_scrub_packet(), sets skb->dev
to the peer in the other netns, and returns -EAGAIN. skb_scrub_packet()
leaves vlan_all alone, and nothing in this cycle counts iterations.

Here is one example setup. A veth pair is split across two netns. Each end
has a VID 100 VLAN device and no VID 0 device. Each VLAN device has an
ingress BPF program that calls bpf_redirect_peer() on its lower veth. A
frame carrying repeated [VID 0, VID 100] tag pairs could then cycle like
this:

  vethA: VID 0 branch untags the next VID 100 tag (vlan_depth == 2)
  vlan_do_receive() finds vlan100A -> goto another_round
  top-of-round skb_vlan_untag() strips the next VID 0 tag
  TC on vlan100A peer-redirects to vethB -> another_round
  vethB: VID 0 branch starts over with vlan_depth == 1
  ...

The same loop also seems possible with VID 100 tags alone. That version
goes through the earlier vlan_do_receive() -> goto another_round before
rx_handler and never reaches the patched branch.

Each round, skb_vlan_untag() calls skb_reset_network_header():

include/linux/skbuff.h:skb_reset_network_header() {
	long offset = skb->data - skb->head;

	DEBUG_NET_WARN_ON_ONCE(offset != (typeof(skb->network_header))offset);
	skb->network_header = offset;
}

After about 16K tags the 16-bit offset wraps. That is the out-of-bounds
condition the commit message describes.

Would it work better to put the bound inside skb_vlan_untag() itself, for
example on the header offset or the headroom? Another option is a depth
count that survives another_round.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261023003750.1133904-1-user148783812897%40gmail.com

Re: [PATCH net] net: limit priority VLAN untag depth

[Kuniyuki Iwashima]

On Tue, 27 Oct 2026 00:40:20 +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 1 · Medium: 0 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [High] The commit message promises a per-packet bound, but the patch
>   only bounds one pass through __netif_receive_skb_core()…
> 
> --
> 
> net: limit priority VLAN untag depth
> 
> This patch adds a vlan_depth counter to the VID 0 retry branch in
> __netif_receive_skb_core(). Once VLAN_MAX_DEPTH tags have been consumed,
> the packet is dropped. The aim is to stop repeated skb_vlan_untag() calls
> from wrapping the 16-bit network and transport header offsets.
> 
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index e76762e29360e..b40efea27dc29 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -6198,6 +6198,8 @@ static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
> >  	}
> >  
> >  	if (unlikely(skb_vlan_tag_present(skb)) && !netdev_uses_dsa(skb->dev)) {
> > +		unsigned int vlan_depth = 1;
> > +
> >  check_vlan_id:
> >  		if (skb_vlan_tag_get_id(skb)) {
> >  			/* Vlan id is non 0 and vlan_do_receive() above couldn't
> > @@ -6209,6 +6211,10 @@ static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
> >  			 * 802.1Q or 802.1AD and vlan_do_receive() above could
> >  			 * not find vlan dev for vlan id 0.
> >  			 */
> > +			if (unlikely(vlan_depth >= VLAN_MAX_DEPTH))
> > +				goto drop;
> > +
> > +			vlan_depth++;
> >  			__vlan_hwaccel_clear_tag(skb);
> >  			skb = skb_vlan_untag(skb);
> >  			if (unlikely(!skb))
> 
> [Severity: High]
> 
> Does this limit the number of tags removed per packet, or only per pass
> through __netif_receive_skb_core()?
> 
> vlan_depth is declared inside the if block, so it goes back to 1 every
> time processing jumps to another_round. The VID 0 branch itself leaves
> through another_round when the inner tag matches a VLAN device:
> 
> 			if (vlan_do_receive(&skb))
> 				/* After stripping off 802.1P header with vlan 0
> 				 * vlan dev is found for inner header.
> 				 */
> 				goto another_round;
> 
> A TC ingress peer redirect also restarts the round on the same skb, and
> skb->data does not change:
> 
> net/core/dev.c:sch_handle_ingress() {
>     ...
> 	__skb_push(skb, skb->mac_len);
> 	if (skb_do_redirect(skb) == -EAGAIN) {
> 		__skb_pull(skb, skb->mac_len);
> 		*another = true;
> 		break;
> 	}
>     ...
> }
> 
> With BPF_F_PEER, skb_do_redirect() calls skb_scrub_packet(), sets skb->dev
> to the peer in the other netns, and returns -EAGAIN. skb_scrub_packet()
> leaves vlan_all alone, and nothing in this cycle counts iterations.
> 
> Here is one example setup. A veth pair is split across two netns. Each end
> has a VID 100 VLAN device and no VID 0 device. Each VLAN device has an
> ingress BPF program that calls bpf_redirect_peer() on its lower veth. A
> frame carrying repeated [VID 0, VID 100] tag pairs could then cycle like
> this:
> 
>   vethA: VID 0 branch untags the next VID 100 tag (vlan_depth == 2)
>   vlan_do_receive() finds vlan100A -> goto another_round
>   top-of-round skb_vlan_untag() strips the next VID 0 tag
>   TC on vlan100A peer-redirects to vethB -> another_round
>   vethB: VID 0 branch starts over with vlan_depth == 1
>   ...
> 
> The same loop also seems possible with VID 100 tags alone. That version
> goes through the earlier vlan_do_receive() -> goto another_round before
> rx_handler and never reaches the patched branch.
> 
> Each round, skb_vlan_untag() calls skb_reset_network_header():
> 
> include/linux/skbuff.h:skb_reset_network_header() {
> 	long offset = skb->data - skb->head;
> 
> 	DEBUG_NET_WARN_ON_ONCE(offset != (typeof(skb->network_header))offset);
> 	skb->network_header = offset;
> }
> 
> After about 16K tags the 16-bit offset wraps. That is the out-of-bounds
> condition the commit message describes.
> 
> Would it work better to put the bound inside skb_vlan_untag() itself, for
> example on the header offset or the headroom? Another option is a depth
> count that survives another_round.

I have a complimentary patch that implements the latter,
and this change itself looks good to me.


> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261023003750.1133904-1-user148783812897%40gmail.com