Re: [PATCH] mdev: fix daemon crash on reattach mdevs

[Erik Skultety]

On Tue, Jul 21, 2020 at 05:21:10PM +0800, Binfeng Wu wrote:
> Causing a crash when virMediatedDeviceListFindIndex because of
> some pointers in mgr->activeMediatedHostdevs become dangling 
> pointers if goto cleanup label in virMediatedDeviceListMarkDevices.
> 
> Reproduction scenario:
> 1. start vm1 with mdev1
> 2. start vm2 with mdev2, mdev1 (the order cannot be changed)
> 
> Backtrace:
> #0  0xb8c36250 in strcmp
> #1  0xb9b80754 in virMediatedDeviceListFindIndex
> #2  0xb9b80870 in virMediatedDeviceListFind
> #3  0xb9c9e168 in virHostdevReAttachMediatedDevices
> #4  0x9949f724 in qemuHostdevReAttachMediatedDevices
> #5  0x9949f7f8 in qemuHostdevReAttachDomainDevices
> #6  0x994bcd70 in qemuProcessStop
> #7  0x994bf4e0 in qemuProcessStart 

Sorry for the delay, I got my hands on a machine to investigate. Good
catch, it was a tricky one :).

I reworded the commit message a bit to provide more detailed info about
the bug and pushed.

Reviewed-by: Erik Skultety 

[PATCH] mdev: fix daemon crash on reattach mdevs

[Binfeng Wu]

Causing a crash when virMediatedDeviceListFindIndex because of
some pointers in mgr->activeMediatedHostdevs become dangling 
pointers if goto cleanup label in virMediatedDeviceListMarkDevices.

Reproduction scenario:
1. start vm1 with mdev1
2. start vm2 with mdev2, mdev1 (the order cannot be changed)

Backtrace:
#0  0xb8c36250 in strcmp
#1  0xb9b80754 in virMediatedDeviceListFindIndex
#2  0xb9b80870 in virMediatedDeviceListFind
#3  0xb9c9e168 in virHostdevReAttachMediatedDevices
#4  0x9949f724 in qemuHostdevReAttachMediatedDevices
#5  0x9949f7f8 in qemuHostdevReAttachDomainDevices
#6  0x994bcd70 in qemuProcessStop
#7  0x994bf4e0 in qemuProcessStart 
.

Signed-off-by: Binfeng Wu 
---
 src/util/virmdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virmdev.c b/src/util/virmdev.c
index b8023dd991..26cb8300ff 100644
--- a/src/util/virmdev.c
+++ b/src/util/virmdev.c
@@ -439,7 +439,7 @@ virMediatedDeviceListMarkDevices(virMediatedDeviceListPtr 
dst,
 
 if (virMediatedDeviceIsUsed(mdev, dst) ||
 virMediatedDeviceSetUsedBy(mdev, drvname, domname) < 0)
-goto cleanup;
+goto rollback;
 
 /* Copy mdev references to the driver list:
  * - caller is responsible for NOT freeing devices in @src on success
-- 
2.26.2.windows.1