Prevent winbind idmap corruption

[Michael Steffens]

Hi,

the attached patch prevents winbindd from corrupting the
id mapping database in case of write failure. For example when
the filesystem hosting the TDB file is full.

Storing a new meapping consists of three steps

  1. allocate UID/GID (increment HWM)
  2. store mapping UID/GID : SID
  3. store reverse mapping SID : UID/GID

which should be done as a transaction, either completely or not
at all.

The present winbindd_idmap.c does not check success of the
operations above, and will result in an inconsistent mapping
database when any of them fails.

The patched version does check success, and rolls back in
case of failure.

It's not 100% failure proof (transaction is not atomic), but
better than before IMO. :)

Michael

-------------- next part --------------
Index: nsswitch/winbindd_idmap.c
===================================================================
RCS file: /cvsroot/samba/source/nsswitch/winbindd_idmap.c,v
retrieving revision 1.3.4.13
diff -u -r1.3.4.13 winbindd_idmap.c
--- nsswitch/winbindd_idmap.c	27 Apr 2002 03:04:08 -0000	1.3.4.13
+++ nsswitch/winbindd_idmap.c	18 Dec 2002 14:51:08 -0000
@@ -44,6 +44,8 @@
 
     if ((hwm = tdb_fetch_int32(idmap_tdb, 
                              isgroup ? HWM_GROUP : HWM_USER)) == -1) {
+        DEBUG(0, ("Failed to fetch %s : %s\n", isgroup ? HWM_GROUP : HWM_USER,
+            tdb_errorstr(idmap_tdb)));
         return False;
     }
 
@@ -63,7 +65,45 @@
 
     /* Store new high water mark */
 
-    tdb_store_int32(idmap_tdb, isgroup ? HWM_GROUP : HWM_USER, hwm);
+    if (tdb_store_int32(idmap_tdb, isgroup ? HWM_GROUP : HWM_USER, hwm)) {
+        DEBUG(0, ("Failed to store %s %d : %s\n", isgroup ? HWM_GROUP : HWM_USER,
+            hwm, tdb_errorstr(idmap_tdb)));
+        return False;
+    }
+
+    return True;
+}
+
+/* Deallocate either a user or group id, used for failure rollback */
+
+static BOOL deallocate_id(uid_t id, BOOL isgroup)
+{
+    int hwm;
+
+    /* Get current high water mark */
+
+    if ((hwm = tdb_fetch_int32(idmap_tdb, 
+                             isgroup ? HWM_GROUP : HWM_USER)) == -1) {
+        DEBUG(0, ("Failed to fetch %s : %s\n", isgroup ? HWM_GROUP : HWM_USER,
+            tdb_errorstr(idmap_tdb)));
+        return False;
+    }
+
+    if (hwm != id + 1) {
+        /* Should actually never happen, internal redundancy... */
+        DEBUG(0, ("winbind %s mismatch on deallocation!\n", isgroup ? HWM_GROUP : HWM_USER));
+        return False;
+    }
+
+    hwm--;
+
+    /* Store new high water mark */
+
+    if (tdb_store_int32(idmap_tdb, isgroup ? HWM_GROUP : HWM_USER, hwm)) {
+        DEBUG(0, ("Failed to store %s %d : %s\n", isgroup ? HWM_GROUP : HWM_USER,
+           hwm, tdb_errorstr(idmap_tdb)));
+        return False;
+    }
 
     return True;
 }
@@ -109,16 +149,37 @@
             fstring keystr2;
 
             /* Store new id */
-            
+
             slprintf(keystr2, sizeof(keystr2), "%s %d", isgroup ? "GID" : "UID", *id);
 
             data.dptr = keystr2;
             data.dsize = strlen(keystr2) + 1;
 
-            tdb_store(idmap_tdb, key, data, TDB_REPLACE);
-            tdb_store(idmap_tdb, data, key, TDB_REPLACE);
+            /* If any of the following actions fails try to
+               revert modifications successfully made so far. */
 
             result = True;
+
+            if (result && tdb_store(idmap_tdb, key, data, TDB_REPLACE)) {
+                DEBUG(0, ("Failed to store id mapping %s:%s : %s\n",
+                          key.dptr, data.dptr, tdb_errorstr(idmap_tdb)));
+
+                if (!deallocate_id(*id, isgroup))
+                    DEBUG(0, ("Failed to rollback id mapping\n"));
+
+                result = False;
+            }
+
+            if (result && tdb_store(idmap_tdb, data, key, TDB_REPLACE)) {
+                DEBUG(0, ("Failed to store reverse id mapping %s:%s : %s\n",
+                          data.dptr, key.dptr, tdb_errorstr(idmap_tdb)));
+
+                if (!deallocate_id(*id, isgroup) || tdb_delete(idmap_tdb, key))
+                    DEBUG(0, ("Failed to rollback id mapping\n"));
+
+                tdb_delete(idmap_tdb, key);
+                result = False;
+            }
         }
     }