So far, we only process NIC_RX_FILTER_CHANGED event when the
corresponding device has 'trustGuestRxFilters' enabled. And the
event is emitted only for virtio model. IOW, this is fairly
limited situation and other scenarios don't emit any event (e.g.
change of MAC address on a PCI passthrough device).
Resolves: https://issues.redhat.com/browse/RHEL-7035
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/qemu/qemu_domain.c | 16 +++++++++++++++-
src/qemu/qemu_domain.h | 3 ++-
src/qemu/qemu_driver.c | 9 ++++++---
src/qemu/qemu_process.c | 2 +-
4 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 47ae59d408..9dc0a03849 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11018,7 +11018,8 @@ syncNicRxFilterMulticast(char *ifname,
int
qemuDomainSyncRxFilter(virDomainObj *vm,
virDomainNetDef *def,
- virDomainAsyncJob asyncJob)
+ virDomainAsyncJob asyncJob,
+ virObjectEvent **event)
{
qemuDomainObjPrivate *priv = vm->privateData;
g_autoptr(virNetDevRxFilter) guestFilter = NULL;
@@ -11085,6 +11086,19 @@ qemuDomainSyncRxFilter(virDomainObj *vm,
} else {
VIR_FREE(def->guestAddress);
}
+
+ if (event) {
+ char oldMAC[VIR_MAC_STRING_BUFLEN] = { 0 };
+ char newMAC[VIR_MAC_STRING_BUFLEN] = { 0 };
+
+ virMacAddrFormat(&def->mac, oldMAC);
+ virMacAddrFormat(&guestFilter->mac, newMAC);
+
+ *event = virDomainEventNICMACChangeNewFromObj(vm,
+ def->info.alias,
+ oldMAC,
+ newMAC);
+ }
}
return 0;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 8e53a270a7..f3cff49e96 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1128,7 +1128,8 @@ qemuDomainRefreshStatsSchema(virDomainObj *dom);
int
qemuDomainSyncRxFilter(virDomainObj *vm,
virDomainNetDef *def,
- virDomainAsyncJob asyncJob);
+ virDomainAsyncJob asyncJob,
+ virObjectEvent **event);
int
qemuDomainSchedCoreStart(virQEMUDriverConfig *cfg,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a4866450fc..ab1e63a46a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3667,9 +3667,11 @@ processNetdevStreamDisconnectedEvent(virDomainObj *vm,
static void
-processNicRxFilterChangedEvent(virDomainObj *vm,
+processNicRxFilterChangedEvent(virQEMUDriver *driver,
+ virDomainObj *vm,
const char *devAlias)
{
+ virObjectEvent *event = NULL;
virDomainDeviceDef dev;
virDomainNetDef *def;
@@ -3714,11 +3716,12 @@ processNicRxFilterChangedEvent(virDomainObj *vm,
VIR_DEBUG("process NIC_RX_FILTER_CHANGED event for network "
"device %s in domain %s", def->info.alias, vm->def->name);
- if (qemuDomainSyncRxFilter(vm, def, VIR_ASYNC_JOB_NONE) < 0)
+ if (qemuDomainSyncRxFilter(vm, def, VIR_ASYNC_JOB_NONE, &event) < 0)
goto endjob;
endjob:
virDomainObjEndJob(vm);
+ virObjectEventStateQueue(driver->domainEventState, event);
}
@@ -4062,7 +4065,7 @@ static void qemuProcessEventHandler(void *data, void *opaque)
processNetdevStreamDisconnectedEvent(vm, processEvent->data);
break;
case QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED:
- processNicRxFilterChangedEvent(vm, processEvent->data);
+ processNicRxFilterChangedEvent(driver, vm, processEvent->data);
break;
case QEMU_PROCESS_EVENT_SERIAL_CHANGED:
processSerialChangedEvent(driver, vm, processEvent->data,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0173fbe3be..1d7579509a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8199,7 +8199,7 @@ qemuProcessRefreshRxFilters(virDomainObj *vm,
continue;
}
- if (qemuDomainSyncRxFilter(vm, def, asyncJob) < 0)
+ if (qemuDomainSyncRxFilter(vm, def, asyncJob, NULL) < 0)
return -1;
}
--
2.48.1On Mon, Mar 17, 2025 at 12:28:50PM +0100, Michal Privoznik via Devel wrote:
>So far, we only process NIC_RX_FILTER_CHANGED event when the
>corresponding device has 'trustGuestRxFilters' enabled. And the
>event is emitted only for virtio model. IOW, this is fairly
>limited situation and other scenarios don't emit any event (e.g.
>change of MAC address on a PCI passthrough device).
>
>Resolves: https://issues.redhat.com/browse/RHEL-7035
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/qemu/qemu_domain.c | 16 +++++++++++++++-
> src/qemu/qemu_domain.h | 3 ++-
> src/qemu/qemu_driver.c | 9 ++++++---
> src/qemu/qemu_process.c | 2 +-
> 4 files changed, 24 insertions(+), 6 deletions(-)
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index 47ae59d408..9dc0a03849 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -11018,7 +11018,8 @@ syncNicRxFilterMulticast(char *ifname,
> int
> qemuDomainSyncRxFilter(virDomainObj *vm,
> virDomainNetDef *def,
>- virDomainAsyncJob asyncJob)
>+ virDomainAsyncJob asyncJob,
>+ virObjectEvent **event)
> {
> qemuDomainObjPrivate *priv = vm->privateData;
> g_autoptr(virNetDevRxFilter) guestFilter = NULL;
>@@ -11085,6 +11086,19 @@ qemuDomainSyncRxFilter(virDomainObj *vm,
> } else {
> VIR_FREE(def->guestAddress);
If the mac address changed *to* the same one that is configured this is
free'd, but oldMAC still points to it.
> }
>+
>+ if (event) {
>+ char oldMAC[VIR_MAC_STRING_BUFLEN] = { 0 };
>+ char newMAC[VIR_MAC_STRING_BUFLEN] = { 0 };
>+
>+ virMacAddrFormat(&def->mac, oldMAC);
And then in such case this is use after free.
>+ virMacAddrFormat(&guestFilter->mac, newMAC);
>+
>+ *event = virDomainEventNICMACChangeNewFromObj(vm,
>+ def->info.alias,
>+ oldMAC,
>+ newMAC);
>+ }
> }
>
> return 0;
On 3/17/25 14:57, Martin Kletzander wrote:
> On Mon, Mar 17, 2025 at 12:28:50PM +0100, Michal Privoznik via Devel wrote:
>> So far, we only process NIC_RX_FILTER_CHANGED event when the
>> corresponding device has 'trustGuestRxFilters' enabled. And the
>> event is emitted only for virtio model. IOW, this is fairly
>> limited situation and other scenarios don't emit any event (e.g.
>> change of MAC address on a PCI passthrough device).
>>
>> Resolves: https://issues.redhat.com/browse/RHEL-7035
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>> src/qemu/qemu_domain.c | 16 +++++++++++++++-
>> src/qemu/qemu_domain.h | 3 ++-
>> src/qemu/qemu_driver.c | 9 ++++++---
>> src/qemu/qemu_process.c | 2 +-
>> 4 files changed, 24 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 47ae59d408..9dc0a03849 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -11018,7 +11018,8 @@ syncNicRxFilterMulticast(char *ifname,
>> int
>> qemuDomainSyncRxFilter(virDomainObj *vm,
>> virDomainNetDef *def,
>> - virDomainAsyncJob asyncJob)
>> + virDomainAsyncJob asyncJob,
>> + virObjectEvent **event)
>> {
>> qemuDomainObjPrivate *priv = vm->privateData;
>> g_autoptr(virNetDevRxFilter) guestFilter = NULL;
>> @@ -11085,6 +11086,19 @@ qemuDomainSyncRxFilter(virDomainObj *vm,
>> } else {
>> VIR_FREE(def->guestAddress);
>
> If the mac address changed *to* the same one that is configured this is
> free'd, but oldMAC still points to it.
>
>> }
>> +
>> + if (event) {
>> + char oldMAC[VIR_MAC_STRING_BUFLEN] = { 0 };
>> + char newMAC[VIR_MAC_STRING_BUFLEN] = { 0 };
>> +
>> + virMacAddrFormat(&def->mac, oldMAC);
>
> And then in such case this is use after free.
Not really, there's a difference between oldMAC and oldMac O:-)
But I see what you mean, and in fact, that should have been
s/def->mac/oldMac/. I'll post a v2 shortly.
Michal
© 2016 - 2025 Red Hat, Inc.