vlan: annotate data-races in vlan_dev_priv fields

vlan->flags, vlan->ingress_priority_map[] and vlan->nr_*_mappings
are written under RTNL, but read from lockless contexts:

- the RX path reads vlan->flags (VLAN_FLAG_REORDER_HDR) and
  the ingress priority map,
- the TX path reads vlan->flags,
- /proc/net/vlan/<dev> reads vlan->flags and the ingress priority
  map,
- vlan_get_size() reads vlan->nr_*_mappings,
- vlan_fill_info() will soon run without RTNL.

Add the missing READ_ONCE()/WRITE_ONCE() annotations.

vlan_dev_change_flags() is also made a bit more readable by using
a local new_flags variable, avoiding the repeated re-reads of
vlan->flags after it has been published.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20260916132532.1785657-2-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Eric Dumazet 2026-09-16 13:25:31 +00:00 committed by Jakub Kicinski
parent 9211c455de
commit ca6ff8dd70
5 changed files with 35 additions and 28 deletions

View File

@ -158,7 +158,7 @@ static inline u32 vlan_get_ingress_priority(struct net_device *dev,
{ {
struct vlan_dev_priv *vip = vlan_dev_priv(dev); struct vlan_dev_priv *vip = vlan_dev_priv(dev);
return vip->ingress_priority_map[(vlan_tci >> VLAN_PRIO_SHIFT) & 0x7]; return READ_ONCE(vip->ingress_priority_map[(vlan_tci >> VLAN_PRIO_SHIFT) & 0x7]);
} }
#ifdef CONFIG_VLAN_8021Q_GVRP #ifdef CONFIG_VLAN_8021Q_GVRP

View File

@ -38,7 +38,7 @@ bool vlan_do_receive(struct sk_buff **skbp)
skb->pkt_type = PACKET_HOST; skb->pkt_type = PACKET_HOST;
} }
if (!(vlan_dev_priv(vlan_dev)->flags & VLAN_FLAG_REORDER_HDR) && if (!(READ_ONCE(vlan_dev_priv(vlan_dev)->flags) & VLAN_FLAG_REORDER_HDR) &&
!netif_is_macvlan_port(vlan_dev) && !netif_is_macvlan_port(vlan_dev) &&
!netif_is_bridge_port(vlan_dev)) { !netif_is_bridge_port(vlan_dev)) {
unsigned int offset = skb->data - skb_mac_header(skb); unsigned int offset = skb->data - skb_mac_header(skb);

View File

@ -54,7 +54,7 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
u16 vlan_tci = 0; u16 vlan_tci = 0;
int rc; int rc;
if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) { if (!(READ_ONCE(vlan->flags) & VLAN_FLAG_REORDER_HDR)) {
vhdr = skb_push(skb, VLAN_HLEN); vhdr = skb_push(skb, VLAN_HLEN);
vlan_tci = vlan->vlan_id; vlan_tci = vlan->vlan_id;
@ -110,7 +110,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
* NOTE: THIS ASSUMES DIX ETHERNET, SPECIFICALLY NOT SUPPORTING * NOTE: THIS ASSUMES DIX ETHERNET, SPECIFICALLY NOT SUPPORTING
* OTHER THINGS LIKE FDDI/TokenRing/802.3 SNAPs... * OTHER THINGS LIKE FDDI/TokenRing/802.3 SNAPs...
*/ */
if (vlan->flags & VLAN_FLAG_REORDER_HDR || if (READ_ONCE(vlan->flags) & VLAN_FLAG_REORDER_HDR ||
veth->h_vlan_proto != vlan->vlan_proto) { veth->h_vlan_proto != vlan->vlan_proto) {
u16 vlan_tci; u16 vlan_tci;
vlan_tci = vlan->vlan_id; vlan_tci = vlan->vlan_id;
@ -159,13 +159,16 @@ void vlan_dev_set_ingress_priority(const struct net_device *dev,
u32 skb_prio, u16 vlan_prio) u32 skb_prio, u16 vlan_prio)
{ {
struct vlan_dev_priv *vlan = vlan_dev_priv(dev); struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
u32 *map = &vlan->ingress_priority_map[vlan_prio & 0x7];
if (vlan->ingress_priority_map[vlan_prio & 0x7] && !skb_prio) if (*map && !skb_prio)
vlan->nr_ingress_mappings--; WRITE_ONCE(vlan->nr_ingress_mappings,
else if (!vlan->ingress_priority_map[vlan_prio & 0x7] && skb_prio) vlan->nr_ingress_mappings - 1);
vlan->nr_ingress_mappings++; else if (!*map && skb_prio)
WRITE_ONCE(vlan->nr_ingress_mappings,
vlan->nr_ingress_mappings + 1);
vlan->ingress_priority_map[vlan_prio & 0x7] = skb_prio; WRITE_ONCE(*map, skb_prio);
} }
int vlan_dev_set_egress_priority(const struct net_device *dev, int vlan_dev_set_egress_priority(const struct net_device *dev,
@ -185,7 +188,8 @@ int vlan_dev_set_egress_priority(const struct net_device *dev,
if (mp->priority == skb_prio) { if (mp->priority == skb_prio) {
if (!vlan_qos) { if (!vlan_qos) {
rcu_assign_pointer(*mpp, rtnl_dereference(mp->next)); rcu_assign_pointer(*mpp, rtnl_dereference(mp->next));
vlan->nr_egress_mappings--; WRITE_ONCE(vlan->nr_egress_mappings,
vlan->nr_egress_mappings - 1);
kfree_rcu(mp, rcu); kfree_rcu(mp, rcu);
} else { } else {
WRITE_ONCE(mp->vlan_qos, vlan_qos); WRITE_ONCE(mp->vlan_qos, vlan_qos);
@ -209,7 +213,8 @@ int vlan_dev_set_egress_priority(const struct net_device *dev,
RCU_INIT_POINTER(np->next, rtnl_dereference(vlan->egress_priority_map[bucket])); RCU_INIT_POINTER(np->next, rtnl_dereference(vlan->egress_priority_map[bucket]));
rcu_assign_pointer(vlan->egress_priority_map[bucket], np); rcu_assign_pointer(vlan->egress_priority_map[bucket], np);
if (vlan_qos) if (vlan_qos)
vlan->nr_egress_mappings++; WRITE_ONCE(vlan->nr_egress_mappings,
vlan->nr_egress_mappings + 1);
return 0; return 0;
} }
@ -220,23 +225,25 @@ int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
{ {
struct vlan_dev_priv *vlan = vlan_dev_priv(dev); struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
u32 old_flags = vlan->flags; u32 old_flags = vlan->flags;
u32 new_flags;
if (mask & ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP | if (mask & ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP |
VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP | VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP |
VLAN_FLAG_BRIDGE_BINDING)) VLAN_FLAG_BRIDGE_BINDING))
return -EINVAL; return -EINVAL;
vlan->flags = (old_flags & ~mask) | (flags & mask); new_flags = (old_flags & ~mask) | (flags & mask);
WRITE_ONCE(vlan->flags, new_flags);
if (netif_running(dev) && (vlan->flags ^ old_flags) & VLAN_FLAG_GVRP) { if (netif_running(dev) && (new_flags ^ old_flags) & VLAN_FLAG_GVRP) {
if (vlan->flags & VLAN_FLAG_GVRP) if (new_flags & VLAN_FLAG_GVRP)
vlan_gvrp_request_join(dev); vlan_gvrp_request_join(dev);
else else
vlan_gvrp_request_leave(dev); vlan_gvrp_request_leave(dev);
} }
if (netif_running(dev) && (vlan->flags ^ old_flags) & VLAN_FLAG_MVRP) { if (netif_running(dev) && (new_flags ^ old_flags) & VLAN_FLAG_MVRP) {
if (vlan->flags & VLAN_FLAG_MVRP) if (new_flags & VLAN_FLAG_MVRP)
vlan_mvrp_request_join(dev); vlan_mvrp_request_join(dev);
else else
vlan_mvrp_request_leave(dev); vlan_mvrp_request_leave(dev);
@ -599,7 +606,7 @@ void vlan_dev_free_egress_priority(const struct net_device *dev)
pm = next; pm = next;
} }
} }
vlan->nr_egress_mappings = 0; WRITE_ONCE(vlan->nr_egress_mappings, 0);
} }
static void vlan_dev_uninit(struct net_device *dev) static void vlan_dev_uninit(struct net_device *dev)

View File

@ -214,8 +214,8 @@ static size_t vlan_get_size(const struct net_device *dev)
return nla_total_size(2) + /* IFLA_VLAN_PROTOCOL */ return nla_total_size(2) + /* IFLA_VLAN_PROTOCOL */
nla_total_size(2) + /* IFLA_VLAN_ID */ nla_total_size(2) + /* IFLA_VLAN_ID */
nla_total_size(sizeof(struct ifla_vlan_flags)) + /* IFLA_VLAN_FLAGS */ nla_total_size(sizeof(struct ifla_vlan_flags)) + /* IFLA_VLAN_FLAGS */
vlan_qos_map_size(vlan->nr_ingress_mappings) + vlan_qos_map_size(READ_ONCE(vlan->nr_ingress_mappings)) +
vlan_qos_map_size(vlan->nr_egress_mappings); vlan_qos_map_size(READ_ONCE(vlan->nr_egress_mappings));
} }
static int vlan_fill_info(struct sk_buff *skb, const struct net_device *dev) static int vlan_fill_info(struct sk_buff *skb, const struct net_device *dev)

View File

@ -240,7 +240,7 @@ static int vlandev_seq_show(struct seq_file *seq, void *offset)
seq_printf(seq, seq_printf(seq,
"%s VID: %d REORDER_HDR: %i dev->priv_flags: %x\n", "%s VID: %d REORDER_HDR: %i dev->priv_flags: %x\n",
vlandev->name, vlan->vlan_id, vlandev->name, vlan->vlan_id,
(int)(vlan->flags & 1), (u32)vlandev->priv_flags); (int)(READ_ONCE(vlan->flags) & 1), (u32)vlandev->priv_flags);
seq_printf(seq, fmt64, "total frames received", stats->rx_packets); seq_printf(seq, fmt64, "total frames received", stats->rx_packets);
seq_printf(seq, fmt64, "total bytes received", stats->rx_bytes); seq_printf(seq, fmt64, "total bytes received", stats->rx_bytes);
@ -252,14 +252,14 @@ static int vlandev_seq_show(struct seq_file *seq, void *offset)
/* now show all PRIORITY mappings relating to this VLAN */ /* now show all PRIORITY mappings relating to this VLAN */
seq_printf(seq, "\nINGRESS priority mappings: " seq_printf(seq, "\nINGRESS priority mappings: "
"0:%u 1:%u 2:%u 3:%u 4:%u 5:%u 6:%u 7:%u\n", "0:%u 1:%u 2:%u 3:%u 4:%u 5:%u 6:%u 7:%u\n",
vlan->ingress_priority_map[0], READ_ONCE(vlan->ingress_priority_map[0]),
vlan->ingress_priority_map[1], READ_ONCE(vlan->ingress_priority_map[1]),
vlan->ingress_priority_map[2], READ_ONCE(vlan->ingress_priority_map[2]),
vlan->ingress_priority_map[3], READ_ONCE(vlan->ingress_priority_map[3]),
vlan->ingress_priority_map[4], READ_ONCE(vlan->ingress_priority_map[4]),
vlan->ingress_priority_map[5], READ_ONCE(vlan->ingress_priority_map[5]),
vlan->ingress_priority_map[6], READ_ONCE(vlan->ingress_priority_map[6]),
vlan->ingress_priority_map[7]); READ_ONCE(vlan->ingress_priority_map[7]));
seq_printf(seq, " EGRESS priority mappings: "); seq_printf(seq, " EGRESS priority mappings: ");
rcu_read_lock(); rcu_read_lock();