HEADS UP: critical locking bug in bogofilter 0.13.7.4...0.14.2

[Matthias Andree]

THIS MESSAGE DOES NOT APPLY TO BOGOFILTER-STABLE VERSION 0.13.7.2.
------------------------------------------------------------------

A critical bug was found in bogofilter-current from 0.13.7.4 to 0.14.2,
affecting data base locking for separate wordlist mode (that most of you
are using if you installed bogofilter older than and excluding 0.14 for
the first time): since bogofilter 0.13.7.4, bogofilter only locked one
of the two files in separate-wordlist mode. An untested patch against
0.14.2 is below.

Remember that the bogofilter-current releases are development snapshots
that haven't undergone as much testing that bogofilter-stable releases
have been through, so bugs like these are much more likely to bite in
bogofilter-current releases. If you want to use bogofilter for
production, please stick with the bogofilter-stable releases.

All users of bogofilter versions 0.13.7.4 to 0.14.2 are advised to apply
the patch below and recompile and reinstall bogofilter (A "no previous
prototype" warning can safely be ignored, it's harmless.), or to upgrade
to a newer bogofilter-current version (no such release is available at
the time I'm writing this).

After that, checking (db_verify or bogoutil -d) or rebuilding the data
bases or at least dumping and loading them with bogoutil is advised, to
avoid growing of data base corruptions that may have happened.

Index: src/datastore_db.c
===================================================================
RCS file: /cvsroot/bogofilter/bogofilter/src/datastore_db.c,v
retrieving revision 1.32
retrieving revision 1.36
diff -u -r1.32 -r1.36
--- src/datastore_db.c	2 Aug 2003 21:55:43 -0000	1.32
+++ src/datastore_db.c	5 Aug 2003 00:26:02 -0000	1.36
@@ -1,4 +1,4 @@
-/* $Id: datastore_db.c,v 1.32 2003/08/02 21:55:43 relson Exp $ */
+/* $Id: datastore_db.c,v 1.36 2003/08/05 00:26:02 m-a Exp $ */
 
 /*****************************************************************************
 
@@ -21,6 +21,10 @@
 #include <db.h>
 #include <errno.h>
 
+#if NEEDTRIO
+#include <trio.h>
+#endif
+
 #include "datastore.h"
 #include "error.h"
 #include "maint.h"
@@ -33,7 +37,7 @@
     char	*filename;
     size_t	count;
     char	*name[2];
-    int		fd;
+    int		fd[2];
     dbmode_t	open_mode;
     DB		*dbp[2];
     pid_t	pid;
@@ -58,6 +62,15 @@
 
 /* Function definitions */
 
+const char *db_version_str(void)
+{
+    static char v[80];
+    snprintf(v, sizeof(v), "BerkeleyDB (%d.%d.%d)",
+	    DB_VERSION_MAJOR, DB_VERSION_MINOR, DB_VERSION_PATCH);
+    return v;
+}
+
+
 static void db_enforce_locking(dbh_t *handle, const char *func_name)
 {
     if (handle->locked == false) {
@@ -80,12 +93,12 @@
     for (c = 0; c < count ; c += 1) {
 	size_t len = strlen(path) + strlen(names[c]) + 2;
 	handle->name[c] = xmalloc(len);
+	handle->fd[c]    = -1; /* for lock */
 	build_path(handle->name[c], len, path, names[c]);
     }
     handle->pid	    = getpid();
     handle->locked  = false;
     handle->is_swapped= 0;
-    handle->fd	    = -1; /* for lock */
 
     return handle;
 }
@@ -171,7 +184,7 @@
 	if (handle == NULL)
 	    break;
 
-	for (i = 0; i < handle->count; i += 1) {
+	for (i = 0; handle && i < handle->count; i += 1) {
 	    DB *dbp;
 
 	    /* create DB handle */
@@ -217,33 +230,37 @@
 		return NULL;		/* handle already freed, ok to return */
 	    }
 
-	    ret = dbp->fd(dbp, &handle->fd);
+	    ret = dbp->fd(dbp, &handle->fd[i]);
 	    if (ret < 0) {
 		dbp->err (dbp, ret, "%s (db) fd: %s",
 			  progname, handle->name);
 		db_close(handle, false);
 		return NULL;		/* handle already freed, ok to return */
 	    }
-	}
-
-	if (db_lock(handle->fd, F_SETLK,
-		    (short int)(open_mode == DB_READ ? F_RDLCK : F_WRLCK)))
-	{
-	    int e = errno;
-	    handle->fd = -1;
-	    db_close(handle, true);
-	    handle = NULL;	/* db_close freed it, we don't want to use it anymore */
-	    errno = e;
-	    /* do not bother to retry if the problem wasn't EAGAIN */
-	    if (e != EAGAIN && e != EACCES) return NULL;
-	    /* do not goto open_err here, db_close frees the handle! */
-	} else {
-	    break;
+	    if (db_lock(handle->fd[i], F_SETLK,
+			(short int)(open_mode == DB_READ ? F_RDLCK : F_WRLCK)))
+	    {
+		int e = errno;
+		handle->fd[i] = -1;
+		db_close(handle, true);
+		handle = NULL;	/* db_close freed it, we don't want to use it anymore */
+		errno = e;
+		/* do not bother to retry if the problem wasn't EAGAIN */
+		if (e != EAGAIN && e != EACCES) return NULL;
+		/* do not goto open_err here, db_close frees the handle! */
+	    } else {
+		break;
+	    }
 	}
     }
 
-    if (handle && (handle->fd >= 0)) {
+    if (handle) {
+	unsigned int i;
 	handle->locked = true;
+	for (i = 0; i < handle->count; i += 1) {
+	    if (handle->fd[i] < 0) 
+		handle->locked=false;
+	}
 	return (void *)handle;
     }
 
@@ -251,7 +268,7 @@
 
  open_err:
     dbh_free(handle);
-    
+
     return NULL;
 }
 

-- 
Matthias Andree