Fix nscd assertion failure in gc (bug 19755)

If a GETxxBYyy request (for passwd or group) is running in parallel to
an INVALIDATE request (for the same database) then in a particular order
of events the garbage collector is not properly marking all used memory
and fails an assertion:

   GETGRBYNAME (root)
Haven't found "root" in group cache!
add new entry "root" of type GETGRBYNAME for group to cache (first)
handle_request: request received (Version = 2) from PID 7413
   INVALIDATE (group)
pruning group cache; time 9223372036854775807
considering GETGRBYNAME entry "root", timeout 1456763027
add new entry "0" of type GETGRBYGID for group to cache
remove GETGRBYNAME entry "root"
nscd: mem.c:403: gc: Assertion `next_data == &he_data[db->head->nentries]' failed.

Here the first call to cache_add added the GETGRBYNAME entry, which is
immediately marked for collection by prune_cache.  Then the GETGRBYGID
entry is added which shares the data packet with the first entry and
therefore is marked as !first, while the marking look in prune_cache has
already finished.  When the garbage collector runs, it only considers
references by entries marked as first, missing the reference by the
secondary entry.

The only way to fix that is to prevent prune_cache from running while the
two related entries are added.

diff --git a/ChangeLog b/ChangeLog
index 58b05e8a48f3bd4db6559459ef40bda68cae2d58..fd472f9577982f6075bfedc530d392b5140ba71d 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2016-06-09  Andreas Schwab  <schwab@suse.de>
+
+       [BZ #19755]
+       * nscd/pwdcache.c (cache_addpw): Lock prune_run_lock while adding
+       new entries in auto-propagate mode.
+       * nscd/grpcache.c (cache_addgr): Likewise.
+
 2016-06-09  Paul Pluzhnikov  <ppluzhnikov@gmail.com>
 
        * test-skeleton.c (oom_error, xmalloc, xcalloc, xrealloc):

diff --git a/nscd/grpcache.c b/nscd/grpcache.c
index 38311701a7eba9e7a34f8500f04a4e23afc15009..8b9b13df16159bb94695b9393d45acd310efa7f2 100644 (file)
--- a/nscd/grpcache.c
+++ b/nscd/grpcache.c
@@ -205,10 +205,19 @@ cache_addgr (struct database_dyn *db, int fd, request_header *req,
       dataset = NULL;
 
       if (he == NULL)
-       dataset = (struct dataset *) mempool_alloc (db, total + n, 1);
+       {
+         /* Prevent an INVALIDATE request from pruning the data between
+            the two calls to cache_add.  */
+         if (db->propagate)
+           pthread_mutex_lock (&db->prune_run_lock);
+         dataset = (struct dataset *) mempool_alloc (db, total + n, 1);
+       }
 
       if (dataset == NULL)
        {
+         if (he == NULL && db->propagate)
+           pthread_mutex_unlock (&db->prune_run_lock);
+
          /* We cannot permanently add the result in the moment.  But
             we can provide the result as is.  Store the data in some
             temporary memory.  */
@@ -396,6 +405,8 @@ cache_addgr (struct database_dyn *db, int fd, request_header *req,
 
        out:
          pthread_rwlock_unlock (&db->lock);
+         if (he == NULL && db->propagate)
+           pthread_mutex_unlock (&db->prune_run_lock);
        }
     }
 

diff --git a/nscd/pwdcache.c b/nscd/pwdcache.c
index 6dd6746f390d4a7c6dc627dc2b25936979513bd3..5ef8485c2a08dd47317c86f51eca1bf9a650fa90 100644 (file)
--- a/nscd/pwdcache.c
+++ b/nscd/pwdcache.c
@@ -198,10 +198,19 @@ cache_addpw (struct database_dyn *db, int fd, request_header *req,
       dataset = NULL;
 
       if (he == NULL)
-       dataset = (struct dataset *) mempool_alloc (db, total + n, 1);
+       {
+         /* Prevent an INVALIDATE request from pruning the data between
+            the two calls to cache_add.  */
+         if (db->propagate)
+           pthread_mutex_lock (&db->prune_run_lock);
+         dataset = (struct dataset *) mempool_alloc (db, total + n, 1);
+       }
 
       if (dataset == NULL)
        {
+         if (he == NULL && db->propagate)
+           pthread_mutex_unlock (&db->prune_run_lock);
+
          /* We cannot permanently add the result in the moment.  But
             we can provide the result as is.  Store the data in some
             temporary memory.  */
@@ -374,6 +383,8 @@ cache_addpw (struct database_dyn *db, int fd, request_header *req,
 
        out:
          pthread_rwlock_unlock (&db->lock);
+         if (he == NULL && db->propagate)
+           pthread_mutex_unlock (&db->prune_run_lock);
        }
     }