[win-pv-devel] [PATCH 1/2] Avoid double-free hazard in XENBUS_CACHE

[Paul Durrant <paul.durrant@xxxxxxxxxx>]

Currently CachePut() calls CachePutObjectToSlab() without holding the cache
lock. This allows two concurrent calls to subsequently serialize on the lock
and both find Slab->Allocated == 0 (the second one actually testing freed
memory), leading to a double-free.

Moving the lock acquisition to before the call to CachePutObjectToSlab()
fixes this problem.

For consistency, this patch also makes it a requirement that
CachePutObjectToSlab() is called with the lock held, and adjusts
__CacheFlushMagazines() accordingly.

Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
---
 src/xenbus/cache.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/xenbus/cache.c b/src/xenbus/cache.c
index a0f4135..877b395 100644
--- a/src/xenbus/cache.c
+++ b/src/xenbus/cache.c
@@ -355,7 +355,7 @@ CacheGetObjectFromSlab(
     return (PVOID)&Slab->Buffer[Index * Cache->Size];
 }
 
-// May be called with or without lock held
+// Must be called with lock held
 static VOID
 CachePutObjectToSlab(
     IN  PXENBUS_CACHE_SLAB  Slab,
@@ -460,11 +460,11 @@ CachePut(
     Slab = (PXENBUS_CACHE_SLAB)PAGE_ALIGN(Object);
     ASSERT3U(Slab->Magic, ==, XENBUS_CACHE_SLAB_MAGIC);
 
-    CachePutObjectToSlab(Slab, Object);
-
     if (!Locked)
         __CacheAcquireLock(Cache);
 
+    CachePutObjectToSlab(Slab, Object);
+
     if (Slab->Allocated == 0) {
         CacheDestroySlab(Cache, Slab);
     } else {
@@ -554,8 +554,12 @@ __CacheFlushMagazines(
     IN  PXENBUS_CACHE   Cache
     )
 {
+    KIRQL               Irql;
     ULONG               Index;
 
+    KeRaiseIrql(DISPATCH_LEVEL, &Irql);
+    __CacheAcquireLock(Cache);
+
     for (Index = 0; Index < Cache->MagazineCount; Index++) {
         PXENBUS_CACHE_MAGAZINE  Magazine = &Cache->Magazine[Index];
         PVOID                   Object;
@@ -569,6 +573,9 @@ __CacheFlushMagazines(
             CachePutObjectToSlab(Slab, Object);
         }
     }
+
+    __CacheReleaseLock(Cache);
+    KeLowerIrql(Irql);
 }
 
 static NTSTATUS
-- 
2.5.3


_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/win-pv-devel