All three Release() implementations checked the member m_nRefCount
after InterlockedDecrement instead of using the local return value.
In a multi-threaded context (vssvc), two threads decrementing
concurrently could both read m_nRefCount as 0 and double-free.

Fixes: b39297aedfab ("qemu-ga: Add Windows VSS provider and requester as DLL")
Signed-off-by: Marc-André Lureau <[email protected]>
---
 qga/vss-win32/provider.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qga/vss-win32/provider.cpp b/qga/vss-win32/provider.cpp
index 2b5c6f8e8ecd..79906dcc24ae 100644
--- a/qga/vss-win32/provider.cpp
+++ b/qga/vss-win32/provider.cpp
@@ -108,7 +108,7 @@ STDMETHODIMP_(ULONG) CQGAVSSEnumObject::AddRef()
 STDMETHODIMP_(ULONG) CQGAVSSEnumObject::Release()
 {
     long nRefCount = InterlockedDecrement(&m_nRefCount);
-    if (m_nRefCount == 0) {
+    if (nRefCount == 0) {
         delete this;
     }
     return nRefCount;
@@ -244,7 +244,7 @@ STDMETHODIMP_(ULONG) CQGAVssProvider::AddRef()
 STDMETHODIMP_(ULONG) CQGAVssProvider::Release()
 {
     long nRefCount = InterlockedDecrement(&m_nRefCount);
-    if (m_nRefCount == 0) {
+    if (nRefCount == 0) {
         delete this;
     }
     return nRefCount;
@@ -477,7 +477,7 @@ STDMETHODIMP_(ULONG) CQGAVssProviderFactory::AddRef()
 STDMETHODIMP_(ULONG) CQGAVssProviderFactory::Release()
 {
     long nRefCount = InterlockedDecrement(&m_nRefCount);
-    if (m_nRefCount == 0) {
+    if (nRefCount == 0) {
         delete this;
     }
     return nRefCount;

-- 
2.55.0.543.g5ebe2ebe4ea8


Reply via email to