* [PATCH net] vlan: fix REORDER_HDR race between header and xmit paths
@ 2026-05-29 7:30 Yizhou Zhao
0 siblings, 0 replies; 3+ messages in thread
From: Yizhou Zhao @ 2026-05-29 7:30 UTC (permalink / raw)
To: netdev
Cc: Yizhou Zhao, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Kees Cook, linux-kernel, Yuxiang Yang,
Ao Wang, Xuewei Feng, Qi Li, Ke Xu
vlan_dev_change_flags() updates vlan->flags under RTNL, but the VLAN
data path reads the same field without RTNL. In particular,
vlan_dev_hard_header() and vlan_dev_hard_start_xmit() may observe
different values of VLAN_FLAG_REORDER_HDR for the same skb.
This can lead to inconsistent tagging. If REORDER_HDR is cleared when
vlan_dev_hard_header() runs, the function pushes an in-band VLAN header
into the skb. If REORDER_HDR is then observed as set by
vlan_dev_hard_start_xmit(), the xmit path may also attach a hardware
accelerated VLAN tag, causing the packet to be emitted with two VLAN
tags. Conversely, if the flag changes in the other direction, the skb
may be emitted without the expected VLAN tag.
Avoid making the xmit decision depend on a second unsynchronized read of
vlan->flags. Instead, use the packet contents already produced by
vlan_dev_hard_header(): when an in-band VLAN header was pushed,
veth->h_vlan_proto matches vlan->vlan_proto and no additional hwaccel tag
is needed; otherwise the packet still carries the encapsulated protocol
and the xmit path must add the VLAN tag.
Also use READ_ONCE() for the data-path read in vlan_dev_hard_header()
and WRITE_ONCE() for the control-path update in vlan_dev_change_flags().
Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
Reported-by: Ao Wang <wangao@seu.edu.cn>
Reported-by: Xuewei Feng <fengxw06@126.com>
Reported-by: Qi Li <qli01@tsinghua.edu.cn>
Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
---
net/8021q/vlan_dev.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index c40f7d5..30cabf7 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -54,7 +54,7 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
u16 vlan_tci = 0;
int rc;
- if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) {
+ if (!(READ_ONCE(vlan->flags) & VLAN_FLAG_REORDER_HDR)) {
vhdr = skb_push(skb, VLAN_HLEN);
vlan_tci = vlan->vlan_id;
@@ -109,9 +109,18 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
*
* NOTE: THIS ASSUMES DIX ETHERNET, SPECIFICALLY NOT SUPPORTING
* OTHER THINGS LIKE FDDI/TokenRing/802.3 SNAPs...
+ *
+ * Do not re-read VLAN_FLAG_REORDER_HDR here. The flag may have been
+ * changed after vlan_dev_hard_header() ran, so using it again can make
+ * the xmit path disagree with the header path for the same skb.
+ *
+ * Instead, look at the packet contents produced by the header path. If
+ * an in-band VLAN header was pushed, h_vlan_proto already matches the
+ * VLAN protocol and no hwaccel tag is needed. Otherwise h_vlan_proto is
+ * the encapsulated protocol and the tag must be added here.
*/
- if (vlan->flags & VLAN_FLAG_REORDER_HDR ||
- veth->h_vlan_proto != vlan->vlan_proto) {
+ if (veth->h_vlan_proto != vlan->vlan_proto) {
u16 vlan_tci;
+
vlan_tci = vlan->vlan_id;
vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb->priority);
__vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
@@ -223,7 +232,7 @@ int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
VLAN_FLAG_BRIDGE_BINDING))
return -EINVAL;
- vlan->flags = (old_flags & ~mask) | (flags & mask);
+ WRITE_ONCE(vlan->flags, (old_flags & ~mask) | (flags & mask));
if (netif_running(dev) && (vlan->flags ^ old_flags) & VLAN_FLAG_GVRP) {
if (vlan->flags & VLAN_FLAG_GVRP)
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH net] vlan: fix REORDER_HDR race between header and xmit paths
@ 2026-05-29 7:31 Yizhou Zhao
2026-06-02 2:04 ` Jakub Kicinski
0 siblings, 1 reply; 3+ messages in thread
From: Yizhou Zhao @ 2026-05-29 7:31 UTC (permalink / raw)
To: netdev
Cc: Yizhou Zhao, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Kees Cook, linux-kernel, Yuxiang Yang,
Ao Wang, Xuewei Feng, Qi Li, Ke Xu
vlan_dev_change_flags() updates vlan->flags under RTNL, but the VLAN
data path reads the same field without RTNL. In particular,
vlan_dev_hard_header() and vlan_dev_hard_start_xmit() may observe
different values of VLAN_FLAG_REORDER_HDR for the same skb.
This can lead to inconsistent tagging. If REORDER_HDR is cleared when
vlan_dev_hard_header() runs, the function pushes an in-band VLAN header
into the skb. If REORDER_HDR is then observed as set by
vlan_dev_hard_start_xmit(), the xmit path may also attach a hardware
accelerated VLAN tag, causing the packet to be emitted with two VLAN
tags. Conversely, if the flag changes in the other direction, the skb
may be emitted without the expected VLAN tag.
Avoid making the xmit decision depend on a second unsynchronized read of
vlan->flags. Instead, use the packet contents already produced by
vlan_dev_hard_header(): when an in-band VLAN header was pushed,
veth->h_vlan_proto matches vlan->vlan_proto and no additional hwaccel tag
is needed; otherwise the packet still carries the encapsulated protocol
and the xmit path must add the VLAN tag.
Also use READ_ONCE() for the data-path read in vlan_dev_hard_header()
and WRITE_ONCE() for the control-path update in vlan_dev_change_flags().
Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
Reported-by: Ao Wang <wangao@seu.edu.cn>
Reported-by: Xuewei Feng <fengxw06@126.com>
Reported-by: Qi Li <qli01@tsinghua.edu.cn>
Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
Assisted-by: GLM:GLM-5.1
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
---
net/8021q/vlan_dev.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index c40f7d5..30cabf7 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -54,7 +54,7 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
u16 vlan_tci = 0;
int rc;
- if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) {
+ if (!(READ_ONCE(vlan->flags) & VLAN_FLAG_REORDER_HDR)) {
vhdr = skb_push(skb, VLAN_HLEN);
vlan_tci = vlan->vlan_id;
@@ -109,9 +109,18 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
*
* NOTE: THIS ASSUMES DIX ETHERNET, SPECIFICALLY NOT SUPPORTING
* OTHER THINGS LIKE FDDI/TokenRing/802.3 SNAPs...
+ *
+ * Do not re-read VLAN_FLAG_REORDER_HDR here. The flag may have been
+ * changed after vlan_dev_hard_header() ran, so using it again can make
+ * the xmit path disagree with the header path for the same skb.
+ *
+ * Instead, look at the packet contents produced by the header path. If
+ * an in-band VLAN header was pushed, h_vlan_proto already matches the
+ * VLAN protocol and no hwaccel tag is needed. Otherwise h_vlan_proto is
+ * the encapsulated protocol and the tag must be added here.
*/
- if (vlan->flags & VLAN_FLAG_REORDER_HDR ||
- veth->h_vlan_proto != vlan->vlan_proto) {
+ if (veth->h_vlan_proto != vlan->vlan_proto) {
u16 vlan_tci;
+
vlan_tci = vlan->vlan_id;
vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb->priority);
__vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
@@ -223,7 +232,7 @@ int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
VLAN_FLAG_BRIDGE_BINDING))
return -EINVAL;
- vlan->flags = (old_flags & ~mask) | (flags & mask);
+ WRITE_ONCE(vlan->flags, (old_flags & ~mask) | (flags & mask));
if (netif_running(dev) && (vlan->flags ^ old_flags) & VLAN_FLAG_GVRP) {
if (vlan->flags & VLAN_FLAG_GVRP)
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] vlan: fix REORDER_HDR race between header and xmit paths
2026-05-29 7:31 [PATCH net] vlan: fix REORDER_HDR race between header and xmit paths Yizhou Zhao
@ 2026-06-02 2:04 ` Jakub Kicinski
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-06-02 2:04 UTC (permalink / raw)
To: Yizhou Zhao
Cc: netdev, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
Kees Cook, linux-kernel, Yuxiang Yang, Ao Wang, Xuewei Feng,
Qi Li, Ke Xu
On Fri, 29 May 2026 15:31:34 +0800 Yizhou Zhao wrote:
> vlan_dev_change_flags() updates vlan->flags under RTNL, but the VLAN
> data path reads the same field without RTNL. In particular,
> vlan_dev_hard_header() and vlan_dev_hard_start_xmit() may observe
> different values of VLAN_FLAG_REORDER_HDR for the same skb.
Does not apply, please rebase on net and repost.
Please add a Fixes tag when you repost.
--
pw-bot: cr
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-02 2:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-29 7:31 [PATCH net] vlan: fix REORDER_HDR race between header and xmit paths Yizhou Zhao
2026-06-02 2:04 ` Jakub Kicinski
-- strict thread matches above, loose matches on Subject: below --
2026-05-29 7:30 Yizhou Zhao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox