netfilter buglog - Apr 2024 - [Bug 1744] New: Packet corruption occurs when using the nftables vlan pcp set command

https://bugzilla.netfilter.org/show_bug.cgi?id=1744

            Bug ID: 1744
           Summary: Packet corruption occurs when using the nftables vlan
                    pcp set command
           Product: nftables
           Version: 1.0.x
          Hardware: arm
                OS: All
            Status: NEW
          Severity: major
          Priority: P5
         Component: kernel
          Assignee: pablo at netfilter.org
          Reporter: yungan.wang at broadcom.com

Created attachment 740
  --> https://bugzilla.netfilter.org/attachment.cgi?id=740&action=edit
Packet sent and received

[Software Information]
Linux Version: 5.15
nftables:
https://www.netfilter.org/projects/nftables/files/nftables-1.0.9.tar.xz
libnftnl:
https://www.netfilter.org/projects/libnftnl/files/libnftnl-1.2.6.tar.xz

[Test Procedure & Result]
1. Add nftables table and chain to filter bridge packets:
nft add table bridge br_filter
nft 'add chain bridge br_filter Postrouting { type filter hook postrouting
priority filter; policy accept; }'

2. Add nftables rule to match VLAN packets with VLAN PCP 2 and change the VLAN
PCP to 7:
nft flush chain bridge br_filter Postrouting
nft add rule bridge br_filter Postrouting vlan pcp 2 vlan pcp set 7 counter
nft -a list chain bridge br_filter Postrouting

3. Send a packet with VLAN PCP 2 through the Linux bridge. (See attached file:
2024-04-09 tx packet.pcapng)

4. Upon receiving the packet after nftables changes the VLAN PCP, the IP header
of the packet is corrupted. The VLAN PCP value remains unchanged. (See attached
file: 2024-04-09 rx packet.pcapng)

[Analysis]
When receiving packets from the Linux bridge, the VLAN tag is already untagged
and stored in skb->vlan_tci. However, when using the "vlan pcp set"
command,
the incorrect offset is obtained in nft_payload_set_eval(), resulting in packet
corruption. To resolve this issue, "vlan pcp set" should set
skb->vlan_tci
instead of modifying skb->data.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20240410/c6de0ff5/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1744

Yungan Wang <yungan.wang at broadcom.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yungan.wang at broadcom.com

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20240410/f017db7c/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1744

--- Comment #1 from Yungan Wang <yungan.wang at broadcom.com> ---
Hi netfilter team,

I was hoping you could assist in clarifying whether the issue I'm
encountering
is a bug or not.

Thanks and best regards,
Yungan

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20240430/47287d05/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1744

Phil Sutter <phil at nwl.cc> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |phil at nwl.cc

--- Comment #2 from Phil Sutter <phil at nwl.cc> ---
Hi,

(In reply to Yungan Wang from comment #1)
> Hi netfilter team,
> 
> I was hoping you could assist in clarifying whether the issue I'm
> encountering is a bug or not.
It clearly is a bug, but the question is whether it has been fixed already or
not. I see several fixes in respective kernel code in near past, could you
please try to reproduce using a more recent (v6.2 at least) kernel?

Cheers, Phil

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20240502/c95c2b4d/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1744

--- Comment #3 from Pablo Neira Ayuso <pablo at netfilter.org> ---
Hi,

nft_payload mangling is lacking special handling for vlan. Linux stores
decapsulated vlan information in skbuff metadata fields. The matching side of
nft_payload already deals, but nft_payload_set_eval() is lacking this.

I can take a look and prepare a bugfix.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20240502/3e47d278/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1744

--- Comment #4 from Yungan Wang <yungan.wang at broadcom.com> ---
(In reply to Phil Sutter from comment #2)
> Hi,
> 
> (In reply to Yungan Wang from comment #1)
> > Hi netfilter team,
> > 
> > I was hoping you could assist in clarifying whether the issue I'm
> > encountering is a bug or not.
> 
> It clearly is a bug, but the question is whether it has been fixed already
> or not. I see several fixes in respective kernel code in near past, could
> you please try to reproduce using a more recent (v6.2 at least) kernel?
> 
> Cheers, Phil
Hi Phil,

I took a look at the latest version of the kernel's
net/netfilter/nft_payload.c
file:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/netfilter/nft_payload.c?h=v6.9-rc6

It seems that nft_payload_set_eval() still doesn't handle skb->vlan_tci.
Therefore, I believe this issue may still occur in the latest kernel version.

Cheers,
Yungan

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20240503/e138af18/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1744

--- Comment #5 from Yungan Wang <yungan.wang at broadcom.com> ---
(In reply to Pablo Neira Ayuso from comment #3)
> Hi,
> 
> nft_payload mangling is lacking special handling for vlan. Linux stores
> decapsulated vlan information in skbuff metadata fields. The matching side
> of nft_payload already deals, but nft_payload_set_eval() is lacking this.
> 
> I can take a look and prepare a bugfix.
Hi Pablo,

Yes, that's exactly the issue I wanted to report. Thanks for addressing it.

Cheers,
Yungan

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20240503/88aac638/attachment-0001.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1744

--- Comment #6 from Pablo Neira Ayuso <pablo at netfilter.org> ---
Hi,

(In reply to Yungan Wang from comment #5)
> (In reply to Pablo Neira Ayuso from comment #3)
> > Hi,
> > 
> > nft_payload mangling is lacking special handling for vlan. Linux
stores
> > decapsulated vlan information in skbuff metadata fields. The matching
side
> > of nft_payload already deals, but nft_payload_set_eval() is lacking
this.
> > 
> > I can take a look and prepare a bugfix.
> 
> Hi Pablo,
> 
> Yes, that's exactly the issue I wanted to report. Thanks for addressing
it.
I have posted a series:

https://patchwork.ozlabs.org/project/netfilter-devel/list/?series=406238

First patch is requires to make QinQ work, and second patch support for
mangling existing vlan tags (it also works with QinQ).

I am also extending existing tests infrastructure to improve coverage for this.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20240510/55a3af61/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1744

--- Comment #7 from Yungan Wang <yungan.wang at broadcom.com> ---
Hey Pablo,

Thanks for the update! I've checked the patches you posted, and it looks
like
they're exactly what we need to resolve this issue.

Cheers,
Yungan

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20240513/e27b4cc7/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1744

Pablo Neira Ayuso <pablo at netfilter.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Pablo Neira Ayuso <pablo at netfilter.org> ---
Userspace patches:

https://git.netfilter.org/nftables/commit/?id=d1a7e74d1e065d244439fdb0f1c1cba83f921609
https://git.netfilter.org/nftables/commit/?id=77e72df6cb10d5c2e8c90a21236c492b4829ffae
https://git.netfilter.org/nftables/commit/?id=3f3c70948f451127d06afb23e2221ed7e17eb977

kernel patches:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/net/netfilter/nft_payload.c?id=aff5c01fa1284d606f8e7cbdaafeef2511bb46c1
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/net/netfilter/nft_payload.c?id=33c563ebf8d3deed7d8addd20d77398ac737ef9a

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20240604/d6538522/attachment.html>