[PATCH] esx: Fix race condition in esxVI_EnsureSession

[Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>]

When the session has expired then multiple threads can race while
reestablishing it.

This race condition is not that critical as it requires a special usage
pattern to be triggerd. It can only happen when an application doesn't
do API calls for quite some time (the session expires after 30 min
inactivity) and then multiple threads doing simultaneous API calls and
end up doing simultaneous calls to esxVI_EnsureSession.
---
 src/esx/esx_vi.c |   47 +++++++++++++++++++++++++++++++++++------------
 src/esx/esx_vi.h |    4 +++-
 2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 756ff68..73413c5 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -603,6 +603,10 @@ ESX_VI__TEMPLATE__ALLOC(Context)
 /* esxVI_Context_Free */
 ESX_VI__TEMPLATE__FREE(Context,
 {
+    if (item->sessionLock != NULL) {
+        virMutexDestroy(item->sessionLock);
+    }
+
     esxVI_CURL_Free(&item->curl);
     VIR_FREE(item->url);
     VIR_FREE(item->ipAddress);
@@ -610,6 +614,7 @@ ESX_VI__TEMPLATE__FREE(Context,
     VIR_FREE(item->password);
     esxVI_ServiceContent_Free(&item->service);
     esxVI_UserSession_Free(&item->session);
+    VIR_FREE(item->sessionLock);
     esxVI_Datacenter_Free(&item->datacenter);
     esxVI_ComputeResource_Free(&item->computeResource);
     esxVI_HostSystem_Free(&item->hostSystem);
@@ -642,6 +647,17 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url,
         return -1;
     }
 
+    if (VIR_ALLOC(ctx->sessionLock) < 0) {
+        virReportOOMError();
+        return -1;
+    }
+
+    if (virMutexInit(ctx->sessionLock) < 0) {
+        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
+                     _("Could not initialize session mutex"));
+        return -1;
+    }
+
     if (esxVI_RetrieveServiceContent(ctx, &ctx->service) < 0) {
         return -1;
     }
@@ -1558,11 +1574,18 @@ esxVI_EnsureSession(esxVI_Context *ctx)
     esxVI_DynamicProperty *dynamicProperty = NULL;
     esxVI_UserSession *currentSession = NULL;
 
-    if (ctx->session == NULL) {
-        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid call"));
+    if (ctx->sessionLock == NULL) {
+        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid call, no mutex"));
         return -1;
     }
 
+    virMutexLock(ctx->sessionLock);
+
+    if (ctx->session == NULL) {
+        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid call, no session"));
+        goto cleanup;
+    }
+
     if (ctx->hasSessionIsActive) {
         /*
          * Use SessionIsActive to check if there is an active session for this
@@ -1570,7 +1593,7 @@ esxVI_EnsureSession(esxVI_Context *ctx)
          */
         if (esxVI_SessionIsActive(ctx, ctx->session->key,
                                   ctx->session->userName, &active) < 0) {
-            return -1;
+            goto cleanup;
         }
 
         if (active != esxVI_Boolean_True) {
@@ -1578,11 +1601,9 @@ esxVI_EnsureSession(esxVI_Context *ctx)
 
             if (esxVI_Login(ctx, ctx->username, ctx->password, NULL,
                             &ctx->session) < 0) {
-                return -1;
+                goto cleanup;
             }
         }
-
-        return 0;
     } else {
         /*
          * Query the session manager for the current session of this connection
@@ -1624,16 +1645,18 @@ esxVI_EnsureSession(esxVI_Context *ctx)
                            "last login"));
             goto cleanup;
         }
+    }
 
-        result = 0;
+    result = 0;
 
   cleanup:
-        esxVI_String_Free(&propertyNameList);
-        esxVI_ObjectContent_Free(&sessionManager);
-        esxVI_UserSession_Free(&currentSession);
+    virMutexUnlock(ctx->sessionLock);
 
-        return result;
-    }
+    esxVI_String_Free(&propertyNameList);
+    esxVI_ObjectContent_Free(&sessionManager);
+    esxVI_UserSession_Free(&currentSession);
+
+    return result;
 }
 
 
diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h
index 75469ab..e3d096e 100644
--- a/src/esx/esx_vi.h
+++ b/src/esx/esx_vi.h
@@ -185,6 +185,7 @@ int esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared, esxVI_CURL *curl);
  */
 
 struct _esxVI_Context {
+    /* All members are used read-only after esxVI_Context_Connect ... */
     esxVI_CURL *curl;
     char *url;
     char *ipAddress;
@@ -193,7 +194,8 @@ struct _esxVI_Context {
     esxVI_ServiceContent *service;
     esxVI_APIVersion apiVersion;
     esxVI_ProductVersion productVersion;
-    esxVI_UserSession *session;
+    esxVI_UserSession *session; /* ... except the session ... */
+    virMutexPtr sessionLock; /* ... that is protected by this mutex */
     esxVI_Datacenter *datacenter;
     esxVI_ComputeResource *computeResource;
     esxVI_HostSystem *hostSystem;
-- 
1.7.0.4

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list