Patchwork Fix nscd assertion failure in gc (bug 19755)

login
register
mail settings
Submitter Andreas Schwab
Date March 2, 2016, 5:13 p.m.
Message ID <mvmmvqgol9v.fsf@hawking.suse.de>
Download mbox | patch
Permalink /patch/11168/
State New
Headers show

Comments

Andreas Schwab - March 2, 2016, 5:13 p.m.
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.

	[BZ #19755]
	* nscd/mem.c (gc): Consider all references to the same data packet.
---
 nscd/mem.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

Patch

diff --git a/nscd/mem.c b/nscd/mem.c
index 1aafd37..49a3b60 100644
--- a/nscd/mem.c
+++ b/nscd/mem.c
@@ -175,14 +175,15 @@  gc (struct database_dyn *db)
 	  /* This is the hash entry itself.  */
 	  markrange (mark, run, sizeof (struct hashentry));
 
-	  /* Add the information for the data itself.  We do this
-	     only for the one special entry marked with FIRST.  */
-	  if (he[cnt]->first)
-	    {
-	      struct datahead *dh
-		= (struct datahead *) (db->data + he[cnt]->packet);
-	      markrange (mark, he[cnt]->packet, dh->allocsize);
-	    }
+	  /* Add the information for the data itself.  We need to do this
+	     for all entries that point to the same data packet, not only
+	     the one marked as first.  Otherwise if the entry marked as
+	     first has already been collected while the secondary entry
+	     was added we may miss the reference from the latter one.
+	     This can happen when processing an INVALIDATE request.  */
+	  struct datahead *dh
+	    = (struct datahead *) (db->data + he[cnt]->packet);
+	  markrange (mark, he[cnt]->packet, dh->allocsize);
 
 	  run = he[cnt]->next;