Account: Race conditions fixes (like: bz#1061150)

This patch solves:
    * Avoid race conditions with shadow-utils.
    * Avoid race condition with libuser: uid/gid "sharing" amoung users/groups.
    * Fix deadlock in lock.c code.

This patch introduces giant lock which is held for all write operations.

5 files changed, 149 insertions, 87 deletions

diff --git a/src/account/LMI_AccountManagementServiceProvider.c b/src/account/LMI_AccountManagementServiceProvider.c
index d4e51d5..21473ea 100644
--- a/src/account/LMI_AccountManagementServiceProvider.c
+++ b/src/account/LMI_AccountManagementServiceProvider.c
@@ -38,6 +38,8 @@
 #include <unistd.h>
 #include <shadow.h>
 
+#include "lock.h"
+
 // Return values of functions
 // common
 #define RET_OK          0
@@ -54,6 +56,10 @@ static const CMPIBroker* _cb = NULL;
 static void LMI_AccountManagementServiceInitialize(const CMPIContext *ctx)
 {
     lmi_init(provider_name, _cb, ctx, provider_config_defaults);
+    if (init_lock_pools() == 0) {
+        error("Unable to initialize lock pool.");
+        exit (1);
+    }
 }
 
 static CMPIStatus LMI_AccountManagementServiceCleanup(
@@ -271,6 +277,12 @@ KUint32 LMI_AccountManagementService_CreateGroup(
         goto clean;
       }
 
+    char userlock[USERNAME_LEN_MAX] = {0};
+    /* -1 for NULL char */
+    strncpy(userlock, Name->chars, sizeof(userlock) - 1);
+    lmi_debug("Getting giant lock for user: %s", userlock);
+    get_giant_lock();
+
     pwdlockres = lckpwdf();
     if (pwdlockres != 0)
         warn("Cannot acquire passwd file lock\n");
@@ -337,10 +349,14 @@ KUint32 LMI_AccountManagementService_CreateGroup(
 
 clean:
 #undef FAIL
-    if (lue) lu_ent_free(lue);
-    if (luc) lu_end(luc);
     if (pwdlockres == 0)
         ulckpwdf();
+    lmi_debug("Releasing giant lock for user: %s", userlock);
+    release_giant_lock();
+    lmi_debug("Giant lock released for user %s", userlock);
+
+    if (lue) lu_ent_free(lue);
+    if (luc) lu_end(luc);
     return result;
 }
 
@@ -405,6 +421,12 @@ KUint32 LMI_AccountManagementService_CreateAccount(
         goto clean;
       }
 
+    char userlock[USERNAME_LEN_MAX] = {0};
+    /* -1 for NULL char */
+    strncpy(userlock, Name->chars, sizeof(userlock) - 1);
+    lmi_debug("Getting giant lock for user: %s", userlock);
+    get_giant_lock();
+
     pwdlockres = lckpwdf();
     if (pwdlockres != 0)
         warn("Cannot acquire passwd file lock\n");
@@ -586,13 +608,16 @@ output:
 
 clean:
 #undef FAIL
+    if (pwdlockres == 0)
+        ulckpwdf();
+    lmi_debug("Releasing giant lock for user: %s", userlock);
+    release_giant_lock();
+    lmi_debug("Giant lock released for user %s", userlock);
+
     free(group_name);
     if (lue) lu_ent_free(lue);
     if (lue_group) lu_ent_free(lue_group);
     if (luc) lu_end(luc);
-    if (pwdlockres == 0)
-        ulckpwdf();
-
     return result;
 }
 
diff --git a/src/account/LMI_AccountProvider.c b/src/account/LMI_AccountProvider.c
index fa2e316..ca9758a 100644
--- a/src/account/LMI_AccountProvider.c
+++ b/src/account/LMI_AccountProvider.c
@@ -285,8 +285,6 @@ static CMPIStatus LMI_AccountModifyInstance(
     struct lu_ent *lue = NULL;
     struct lu_error *error = NULL;
     GValue val;
-    GValueArray *groups = NULL;
-    guint i = 0;
 
     long last_change;
     date_time_prop expiration, warning, inactive_password, inactive_account;
@@ -305,10 +303,8 @@ static CMPIStatus LMI_AccountModifyInstance(
     char userlock[USERNAME_LEN_MAX] = {0};
     /* -1 for NULL char */
     strncpy(userlock, la.Name.chars, sizeof(userlock) - 1);
-    lmi_debug("Getting lock for user: %s", userlock);
-    if (get_user_lock(userlock) == 0) {
-        KReturn2(_cb, ERR_FAILED, "Unable to obtain lock.");
-    }
+    lmi_debug("Getting giant lock for user: %s", userlock);
+    get_giant_lock();
     pwdlockres = lckpwdf();
     if (pwdlockres != 0)
         warn("Cannot acquire passwd file lock\n");
@@ -316,10 +312,11 @@ static CMPIStatus LMI_AccountModifyInstance(
     luc = lu_start(NULL, lu_user, NULL, NULL, lu_prompt_console_quiet, NULL, &error);
     if (!luc)
       {
-        const int ret = release_user_lock(userlock);
-        lmi_debug("Releasing lock for user: %s. Result: %d", userlock, ret);
         if (pwdlockres == 0)
             ulckpwdf();
+        lmi_debug("Releasing giant lock for user: %s", userlock);
+        release_giant_lock();
+        lmi_debug("Giant lock released for user %s", userlock);
         KReturn2(_cb, ERR_FAILED,
                  "Unable to initialize libuser: %s\n", lu_strerror(error));
       }
@@ -333,23 +330,6 @@ static CMPIStatus LMI_AccountModifyInstance(
         goto fail;
       }
 
-    /* get list of groups and lock them. userlock variable is our stored username */
-    groups = lu_groups_enumerate_by_user (luc, userlock, &error);
-    if (groups == NULL)
-      {
-        rc = CMPI_RC_ERR_NOT_FOUND;
-        asprintf(&errmsg, "Get groups for user %s failed with: %s",
-                          userlock, lu_strerror(error));
-        goto fail;
-      }
-
-    for (i = 0; i < groups->n_values; ++i) {
-        GValue *group_val = g_value_array_get_nth (groups, i);
-        const gchar *const group_str = g_value_get_string(group_val);
-        const int ret = get_group_lock((const char *const) group_str);
-        lmi_debug("Getting lock for group: %s. Result: %d", group_str, ret);
-    }
-
     data = ci->ft->getProperty(ci, "UserPassword", NULL);
     ar = data.value.array;
     if (ar && (arsize = ar->ft->getSize(ar, NULL) > 0))
@@ -427,20 +407,13 @@ static CMPIStatus LMI_AccountModifyInstance(
     last_change = aux_lu_get_long(lue, LU_SHADOWLASTCHANGE);
 
 #define FAIL(msg) \
-  for (i = 0; (groups != NULL) && (i < groups->n_values); ++i) { \
-      GValue *group_val = g_value_array_get_nth (groups, i); \
-      const gchar *const group_str = g_value_get_string(group_val); \
-      const int ret = release_group_lock((const char *const) group_str); \
-      lmi_debug("Releasing lock for group: %s. Result: %d", group_str, ret); \
-  } \
-  g_value_array_free (groups); \
-  groups = NULL; \
-  const int ret = release_user_lock(userlock); \
-  lmi_debug("Releasing lock for user: %s. Result: %d", userlock, ret); \
   lu_end(luc); \
   lu_ent_free(lue); \
   if (pwdlockres == 0) \
       ulckpwdf(); \
+  lmi_debug("Releasing lock for user: %s", userlock); \
+  release_giant_lock(); \
+  lmi_debug("Giant lock released for user %s", userlock); \
   KReturn2(_cb, ERR_FAILED, msg);
     GETDATEVALUE("PasswordExpiration", expiration);
     if (!expiration.null && !expiration.interval)
@@ -529,20 +502,14 @@ static CMPIStatus LMI_AccountModifyInstance(
       }
 
 fail:
-    for (i = 0; (groups != NULL) && (i < groups->n_values); ++i) {
-        GValue *group_val = g_value_array_get_nth (groups, i);
-        const gchar *const group_str = g_value_get_string(group_val);
-        const int ret = release_group_lock((const char *const) group_str);
-        lmi_debug("Releasing lock for group: %s. Result: %d", group_str, ret);
-    }
-    g_value_array_free (groups);
-    groups = NULL;
-    const int ret = release_user_lock(userlock);
-    lmi_debug("Releasing lock for user: %s. Result: %d", userlock, ret);
-    lu_ent_free(lue);
-    lu_end(luc);
     if (pwdlockres == 0)
         ulckpwdf();
+    lmi_debug("Releasing giant lock for user: %s", userlock);
+    release_giant_lock ();
+    lmi_debug("Giant lock released for user %s", userlock);
+
+    lu_ent_free(lue);
+    lu_end(luc);
 
     if (errmsg) {
         CMPIString *errstr = CMNewString(_cb, errmsg, NULL);
@@ -581,22 +548,20 @@ static CMPIrc delete_user(
     char userlock[USERNAME_LEN_MAX] = {0};
     /* -1 for NULL char */
     strncpy(userlock, username, sizeof(userlock) - 1);
-    lmi_debug("Getting lock for user: %s. Result: %d", userlock);
-    if (get_user_lock(userlock) == 0) {
-        asprintf(&errormsg, "Unable to obtain lock.");
-        return CMPI_RC_ERR_FAILED;
-    }
+    lmi_debug("Getting giant lock for user: %s", userlock);
+    get_giant_lock();
     pwdlockres = lckpwdf();
     if (pwdlockres != 0)
         warn("Cannot acquire passwd file lock\n");
 
     luc = lu_start(NULL, 0, NULL, NULL, lu_prompt_console_quiet, NULL, &error);
     if (!luc) {
-        const int ret = release_user_lock(userlock);
-        lmi_debug("Releasing lock for user: %s. Result: %d", userlock, ret);
         if (pwdlockres == 0)
             ulckpwdf();
         asprintf(&errormsg, "Unable to initialize libuser: %s\n", lu_strerror(error));
+        lmi_debug("Releasing giant lock for user: %s", userlock);
+        release_giant_lock();
+        lmi_debug("Giant lock released for user %s", userlock);
         return CMPI_RC_ERR_FAILED;
     }
 
@@ -673,16 +638,14 @@ static CMPIrc delete_user(
     }
 
 clean:
-    /* Because C specification... */
-    {
-        const int ret = release_user_lock(userlock);
-        lmi_debug("Releasing lock for user: %s. Result: %d", userlock, ret);
-    }
+    if (pwdlockres == 0)
+        ulckpwdf();
+    lmi_debug("Releasing giant lock for user: %s", userlock);
+    release_giant_lock();
+    lmi_debug("Giant lock released for user %s", userlock);
     lu_ent_free(lue);
     lu_ent_free(lueg);
     lu_end(luc);
-    if (pwdlockres == 0)
-        ulckpwdf();
 
     return rc;
 }
@@ -795,6 +758,12 @@ KUint32 LMI_Account_ChangePassword(
         goto clean;
     }
 
+    char userlock[USERNAME_LEN_MAX] = {0};
+    /* -1 for NULL char */
+    strncpy(userlock, self->Name.chars, sizeof(userlock) - 1);
+    lmi_debug("Getting giant lock for user: %s", userlock);
+    get_giant_lock();
+
     pwdlockres = lckpwdf();
     if (pwdlockres != 0)
         warn("Cannot acquire passwd file lock\n");
@@ -825,11 +794,15 @@ KUint32 LMI_Account_ChangePassword(
     }
 
 clean:
+    if (pwdlockres == 0)
+        ulckpwdf();
+    lmi_debug("Releasing giant lock for user: %s", userlock);
+    release_giant_lock();
+    lmi_debug("Giant lock released for user %s", userlock);
+
     free(errmsg);
     if(luc) lu_end(luc);
     if(lue) lu_ent_free(lue);
-    if (pwdlockres == 0)
-        ulckpwdf();
     return result;
 }
 
diff --git a/src/account/LMI_GroupProvider.c b/src/account/LMI_GroupProvider.c
index efdbad5..20d93b5 100644
--- a/src/account/LMI_GroupProvider.c
+++ b/src/account/LMI_GroupProvider.c
@@ -33,6 +33,8 @@
 #include "account_globals.h"
 #include "globals.h"
 
+#include "lock.h"
+
 // Return values of functions
 // Delete group
 #define GROUP_NOT_EXIST      4096
@@ -43,6 +45,11 @@ static const CMPIBroker* _cb = NULL;
 static void LMI_GroupInitialize(const CMPIContext *ctx)
 {
     lmi_init(provider_name, _cb, ctx, provider_config_defaults);
+    if (init_lock_pools() == 0) {
+        error("Unable to initialize lock pool.");
+        exit (1);
+    }
+
 }
 
 static CMPIStatus LMI_GroupCleanup(
@@ -163,6 +170,9 @@ static CMPIrc delete_group(
     CMPIrc rc = CMPI_RC_OK;
     int pwdlockres;
 
+    lmi_debug("Getting giant lock for group: %s", groupname);
+    get_giant_lock();
+
     pwdlockres = lckpwdf();
     if (pwdlockres != 0)
         warn("Cannot acquire passwd file lock\n");
@@ -172,6 +182,10 @@ static CMPIrc delete_group(
         asprintf(&errormsg, "Unable to initialize libuser: %s\n", lu_strerror(error));
         if (pwdlockres == 0)
             ulckpwdf();
+        lmi_debug("Releasing giant lock for group: %s", groupname);
+        release_giant_lock();
+        lmi_debug("Giant lock released for group %s", groupname);
+
         return CMPI_RC_ERR_FAILED;
     }
 
@@ -208,6 +222,12 @@ static CMPIrc delete_group(
     }
 
 clean:
+    if (pwdlockres == 0)
+        ulckpwdf();
+    lmi_debug("Releasing giant lock for group: %s", groupname);
+    release_giant_lock();
+    lmi_debug("Giant lock released for group %s", groupname);
+
     if (users)
         g_value_array_free(users);
     if (lueg)
@@ -215,8 +235,6 @@ clean:
     if (lueu)
         lu_ent_free(lueu);
     lu_end(luc);
-    if (pwdlockres == 0)
-        ulckpwdf();
     return rc;
 }
 
diff --git a/src/account/lock.c b/src/account/lock.c
index b18b87d..af4668b 100644
--- a/src/account/lock.c
+++ b/src/account/lock.c
@@ -28,28 +28,50 @@
 
 static lock_pools_t pools;
 
-/* Critical sections lock */
+static pthread_once_t pools_are_initialized = PTHREAD_ONCE_INIT;
+static unsigned int ref_count = 0;
+
+/* Callbacks */
+static void new_pools (void);
+static void free_lock (gpointer lock) __attribute__((nonnull));
+
+/* Critical sections locks */
+typedef enum {
+    USER_LOCK = 1,
+    GROUP_LOCK = 2,
+} lck_type_t;
+
 static inline void LOCK_CSEC_POOLS (void)   { pthread_mutex_lock (&pools.csec); }
 static inline void UNLOCK_CSEC_POOLS (void) { pthread_mutex_unlock (&pools.csec); }
 static inline void LOCK_CSEC_USER (void)    { pthread_mutex_lock (&pools.user_pool.csec); }
 static inline void UNLOCK_CSEC_USER (void)  { pthread_mutex_unlock (&pools.user_pool.csec); }
 static inline void LOCK_CSEC_GROUP (void)   { pthread_mutex_lock (&pools.group_pool.csec); }
 static inline void UNLOCK_CSEC_GROUP (void) { pthread_mutex_unlock (&pools.group_pool.csec); }
+static inline void LOCK_GIANT (void)        { pthread_mutex_lock (&giant_lock.mutex); }
+static inline void UNLOCK_GIANT (void)      { pthread_mutex_unlock (&giant_lock.mutex); }
 
-
-static pthread_once_t pools_are_initialized = PTHREAD_ONCE_INIT;
-static unsigned int ref_count = 0;
-
-/* Callbacks */
-static void new_pools (void);
-static void free_lock (gpointer lock) __attribute__((nonnull));
+static inline void LOCK_BY_TYPE (lck_type_t lck_type)
+{
+    if ( lck_type == GROUP_LOCK ) {
+        LOCK_CSEC_GROUP ();
+    } else if ( lck_type == USER_LOCK ) {
+        LOCK_CSEC_USER ();
+    } else { assert ("Unknown lock type"); }
+}
+static inline void UNLOCK_BY_TYPE (lck_type_t lck_type)
+{
+    if ( lck_type == GROUP_LOCK ) {
+        UNLOCK_CSEC_GROUP ();
+    } else if ( lck_type == USER_LOCK ) {
+        UNLOCK_CSEC_USER ();
+    } else { assert ("Unknown lock type"); }
+}
 
 /* Internal functions */
 static int search_key (lock_pool_t *const pool, const char *const key, lock_t **lck) __attribute__((nonnull));
 static int add_key (lock_pool_t *const pool, const char *const key) __attribute__((nonnull));
 static int release_lock (lock_pool_t *const pool, const char *const key) __attribute__((nonnull));
-static int get_lock (lock_pool_t *const pool, const char *const key) __attribute__((nonnull));
-
+static int get_lock (lock_pool_t *const pool, const char *const key, lck_type_t lck_type) __attribute__((nonnull));
 
 int init_lock_pools (void)
 {
@@ -86,6 +108,10 @@ static void new_pools (void)
     pthread_mutex_init (&pools.group_pool.csec, NULL);
     pthread_mutex_init (&pools.csec, NULL);
 
+    /* initialize giant lock */
+    memset (&giant_lock, 0, sizeof (giant_lock_t));
+    pthread_mutex_init (&giant_lock.mutex, NULL);
+
     pools.initialized = 1;
 }
 
@@ -156,14 +182,24 @@ static int add_key (lock_pool_t *const pool, const char *const key)
     return 1;
 }
 
+void get_giant_lock (void)
+{
+    assert (pools.initialized == 1);
+    LOCK_GIANT ();
+}
+
+void release_giant_lock (void)
+{
+    assert (pools.initialized == 1);
+    UNLOCK_GIANT ();
+}
+
 int get_user_lock (const char *const username)
 {
     assert (pools.initialized == 1);
 
-    LOCK_CSEC_USER ();
     lock_pool_t *const pool = &pools.user_pool;
-    const int ret = get_lock (pool, username);
-    UNLOCK_CSEC_USER ();
+    const int ret = get_lock (pool, username, USER_LOCK);
     return ret;
 }
 
@@ -171,17 +207,17 @@ int get_group_lock (const char *const groupname)
 {
     assert (pools.initialized == 1);
 
-    LOCK_CSEC_GROUP ();
     lock_pool_t *const pool = &pools.group_pool;
-    const int ret = get_lock (pool, groupname);
-    UNLOCK_CSEC_GROUP ();
+    const int ret = get_lock (pool, groupname, GROUP_LOCK);
     return ret;
 }
 
-static int get_lock (lock_pool_t *const pool, const char *const key)
+static int get_lock (lock_pool_t *const pool, const char *const key, lck_type_t lck_type)
 {
     assert (pool != NULL);
 
+    LOCK_BY_TYPE (lck_type);
+
     lock_t *lck = NULL;
     int ret = search_key (pool, key, &lck);
     if ( ret == 1 ) {
@@ -193,10 +229,12 @@ static int get_lock (lock_pool_t *const pool, const char *const key)
 
         /* This will raise warning in covscan.
            But I just want return locked mutex. */
+        UNLOCK_BY_TYPE (lck_type);
         pthread_mutex_lock (mutex);
         return 1;
     }
     /* no keys found - add new key to list */
+    UNLOCK_BY_TYPE (lck_type);
     return (add_key (pool, key));
 }
 
diff --git a/src/account/lock.h b/src/account/lock.h
index ab5827b..2b15494 100644
--- a/src/account/lock.h
+++ b/src/account/lock.h
@@ -32,6 +32,12 @@ typedef struct lock {
     unsigned int instances;
 } lock_t;
 
+typedef struct giant_lock {
+    pthread_mutex_t mutex;
+} giant_lock_t;
+
+giant_lock_t giant_lock;
+
 typedef struct lock_pool {
     GHashTable *hash_table;
     pthread_mutex_t csec;
@@ -51,5 +57,7 @@ int get_user_lock (const char *const username) __attribute__((nonnull));
 int get_group_lock (const char *const groupname) __attribute__((nonnull));
 int release_user_lock (const char *const username) __attribute__((nonnull));
 int release_group_lock (const char *const username) __attribute__((nonnull));
+void get_giant_lock (void);
+void release_giant_lock (void);
 
 #endif /* _LOCK_H */