idmap tdb2: fix inconsistent mappings by checking for race and retrying to fetch...

Michael

diff --git a/source/winbindd/idmap.c b/source/winbindd/idmap.c
index c23919fb188064c46fc3ae8ed7a82a567a82a0f4..2962fe6c1c2e16c4750d400f9373281242b38c43 100644 (file)
--- a/source/winbindd/idmap.c
+++ b/source/winbindd/idmap.c
@@ -705,6 +705,15 @@ NTSTATUS idmap_new_mapping(const struct dom_sid *psid, enum id_type type,
 
        status = dom->methods->set_mapping(dom, &map);
 
+       if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_COLLISION)) {
+               struct id_map *ids[2];
+               DEBUG(5, ("Mapping for %s exists - retrying to map sid\n",
+                         sid_string_dbg(map.sid)));
+               ids[0] = ↦
+               ids[1] = NULL;
+               status = dom->methods->sids_to_unixids(dom, ids);
+       }
+
        if (!NT_STATUS_IS_OK(status)) {
                DEBUG(3, ("Could not store the new mapping: %s\n",
                          nt_errstr(status)));

diff --git a/source/winbindd/idmap_tdb2.c b/source/winbindd/idmap_tdb2.c
index 81553dc9c64ec4a292518ea3760104b9dece307a..a3c867f5bbc6d79fc66bb4f1516a324c92169394 100644 (file)
--- a/source/winbindd/idmap_tdb2.c
+++ b/source/winbindd/idmap_tdb2.c
@@ -790,14 +790,12 @@ static NTSTATUS idmap_tdb2_set_mapping(struct idmap_domain *dom, const struct id
        TDB_DATA data;
        char *ksidstr, *kidstr;
        struct db_record *update_lock = NULL;
-       struct db_record *rec = NULL;
 
        if (!map || !map->sid) {
                return NT_STATUS_INVALID_PARAMETER;
        }
 
        ksidstr = kidstr = NULL;
-       data.dptr = NULL;
 
        /* TODO: should we filter a set_mapping using low/high filters ? */
        
@@ -845,66 +843,25 @@ static NTSTATUS idmap_tdb2_set_mapping(struct idmap_domain *dom, const struct id
                goto done;
        }
 
-       /*
-        * *DELETE* previous mappings if any. *
-        */
-
-       /* First delete indexed on SID */
-
-       if (((rec = idmap_tdb2_perm->fetch_locked(
-                    idmap_tdb2_perm, update_lock,
-                    string_term_tdb_data(ksidstr))) != NULL)
-           && (rec->value.dsize != 0)) {
-               struct db_record *rec2;
-
-               if ((rec2 = idmap_tdb2_perm->fetch_locked(
-                            idmap_tdb2_perm, update_lock, rec->value))
-                   != NULL) {
-                       rec2->delete_rec(rec2);
-                       TALLOC_FREE(rec2);
-               }
-
-               rec->delete_rec(rec);
-
-               tdb_delete(idmap_tdb2_tmp, rec->key);
-               tdb_delete(idmap_tdb2_tmp, rec->value);
-       }
-       TALLOC_FREE(rec);
-
-       /* Now delete indexed on unix ID */
-
-       if (((rec = idmap_tdb2_perm->fetch_locked(
-                    idmap_tdb2_perm, update_lock,
-                    string_term_tdb_data(kidstr))) != NULL)
-           && (rec->value.dsize != 0)) {
-               struct db_record *rec2;
-
-               if ((rec2 = idmap_tdb2_perm->fetch_locked(
-                            idmap_tdb2_perm, update_lock, rec->value))
-                   != NULL) {
-                       rec2->delete_rec(rec2);
-                       TALLOC_FREE(rec2);
-               }
-
-               rec->delete_rec(rec);
-
-               tdb_delete(idmap_tdb2_tmp, rec->key);
-               tdb_delete(idmap_tdb2_tmp, rec->value);
+       /* check wheter sid mapping is already present in db */
+       data = tdb2_fetch_bystring(ksidstr, ksidstr);
+       if (data.dptr) {
+               ret = NT_STATUS_OBJECT_NAME_COLLISION;
+               goto done;
        }
-       TALLOC_FREE(rec);
 
-       if (!NT_STATUS_IS_OK(tdb2_store_bystring(ksidstr, string_term_tdb_data(kidstr),
-                               TDB_INSERT))) {
-               DEBUG(0, ("Error storing SID -> ID\n"));
-               ret = NT_STATUS_UNSUCCESSFUL;
+       ret = tdb2_store_bystring(ksidstr, string_term_tdb_data(kidstr),
+                                 TDB_INSERT);
+       if (!NT_STATUS_IS_OK(ret)) {
+               DEBUG(0, ("Error storing SID -> ID: %s\n", nt_errstr(ret)));
                goto done;
        }
-       if (!NT_STATUS_IS_OK(tdb2_store_bystring(kidstr, string_term_tdb_data(ksidstr),
-                               TDB_INSERT))) {
-               DEBUG(0, ("Error storing ID -> SID\n"));
+       ret = tdb2_store_bystring(kidstr, string_term_tdb_data(ksidstr),
+                                 TDB_INSERT);
+       if (!NT_STATUS_IS_OK(ret)) {
+               DEBUG(0, ("Error storing ID -> SID: %s\n", nt_errstr(ret)));
                /* try to remove the previous stored SID -> ID map */
                tdb2_delete_bystring(ksidstr);
-               ret = NT_STATUS_UNSUCCESSFUL;
                goto done;
        }
 
@@ -914,7 +871,6 @@ static NTSTATUS idmap_tdb2_set_mapping(struct idmap_domain *dom, const struct id
 done:
        talloc_free(ksidstr);
        talloc_free(kidstr);
-       SAFE_FREE(data.dptr);
        TALLOC_FREE(update_lock);
        return ret;
 }