The VLAN filter table was not being properly initialized when the driver
sets the DRIVER_OK status bit, causing incorrect VLAN filtering behavior
that could allow unintended VLAN packets to pass through. Correct the
table initialization timing to fix the issue.
Problem
-------
The expected initial state of the table depends on feature negotiation:
With VIRTIO_NET_F_CTRL_VLAN:
The table must be empty in accordance with the specification.
Without VIRTIO_NET_F_CTRL_VLAN:
The table should be filled to permit all VLAN traffic.
Prior to commit ("virtio-net: do not reset vlan filtering at
set_features"), virtio_net_set_features() always initialized the VLAN
table. That commit changed the behavior to skip initialization when
VIRTIO_NET_F_CTRL_VLAN was negotiated, assuming the table would be
properly cleared during reset and remain stable.
However, this assumption breaks when a driver renegotiates features:
1. Initial negotiation without VIRTIO_NET_F_CTRL_VLAN (table filled)
2. Renegotiation with VIRTIO_NET_F_CTRL_VLAN (table will not be cleared)
The problem was exacerbated by commit 0caed25cd171 ("virtio: Call
set_features during reset"), which triggered virtio_net_set_features()
during reset, exposing the bug whenever VIRTIO_NET_F_CTRL_VLAN was
negotiated after a reset.
Solution
--------
Fix by moving VLAN table initialization to virtio_net_set_status(),
which sets the DRIVER_OK status bit. This ensures proper table
initialization regardless of feature negotiation sequence.
Fixes: 06b636a1e2ad ("virtio-net: do not reset vlan filtering at set_features")
Reported-by: Konstantin Shkolnyy <k...@linux.ibm.com>
Signed-off-by: Akihiko Odaki <od...@rsg.ci.i.u-tokyo.ac.jp>
---
Not tested.
Konstantin, please see if this patch fixes your workload.
---
hw/net/virtio-net.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index
221252e00a50a46033d7ec8d18936e7c8196a6ca..6d2a67471e570e1e9b4b4fb5338d87c30e23ae36
100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -389,6 +389,12 @@ static int virtio_net_set_status(struct VirtIODevice
*vdev, uint8_t status)
int i;
uint8_t queue_status;
+ if ((status & VIRTIO_CONFIG_S_DRIVER_OK) &&
+ !(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+ bool vlan = virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VLAN);
+ memset(n->vlans, vlan ? 0 : 0xff, MAX_VLAN >> 3);
+ }
+
virtio_net_vnet_endian_status(n, status);
virtio_net_vhost_status(n, status);
@@ -998,10 +1004,6 @@ static void virtio_net_set_features(VirtIODevice *vdev,
uint64_t features)
vhost_net_save_acked_features(nc->peer);
}
- if (!virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
- memset(n->vlans, 0xff, MAX_VLAN >> 3);
- }
-
if (virtio_has_feature(features, VIRTIO_NET_F_STANDBY)) {
qapi_event_send_failover_negotiated(n->netclient_name);
qatomic_set(&n->failover_primary_hidden, false);
@@ -3990,7 +3992,6 @@ static void virtio_net_reset(VirtIODevice *vdev)
memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
memcpy(&n->mac[0], &n->nic->conf->macaddr, sizeof(n->mac));
qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
- memset(n->vlans, 0, MAX_VLAN >> 3);
/* Flush any async TX */
for (i = 0; i < n->max_queue_pairs; i++) {
---
base-commit: f0737158b483e7ec2b2512145aeab888b85cc1f7
change-id: 20250713-vlan-8c107a65ad91
Best regards,
--
Akihiko Odaki <od...@rsg.ci.i.u-tokyo.ac.jp>
