[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [leafnode-list] Memory corruption bug in



"Robert J. Clark" <clark@xxxxxxxxxx> writes:

> I have been playing around with the -0710a release and noticed that
> fetchnews was segfaulting after it's run. I got as far as determining
> that the change to PCRE newsgroups is at the heart of the problem.

Indeed, there's a memory allocation issue: the pcre is compiled once and
the pointer to it is copied and freed more than once. Workaround: use
one newsgroup line per action. (I found other memory management bugs
with valgrind as I debugged this.)

> It appears to me as through the code to free the filters loaded from
> /etc/leafnode/filterfile is stepping on the list of local news groups in
> memory.

Nope, it's freeing the same block more than once.

>   (gdb) where
>   #0  0x400cff63 in mallopt () from /lib/i686/libc.so.6
>   #1  0x400cedfc in free () from /lib/i686/libc.so.6
>   #2  0x080525ba in free_entry (e=0x4018bf50) at filterutil.c:598

And that's the exact location where the problem showed.

Could you test this patch against 20030712a and let me know if that
fixes your problem?

Index: NEWS
===================================================================
RCS file: /var/CVS/leafnode-2/NEWS,v
retrieving revision 1.20
diff -u -r1.20 NEWS
--- NEWS	12 Jul 2003 23:58:10 -0000	1.20
+++ NEWS	13 Jul 2003 00:59:36 -0000
@@ -1,6 +1,11 @@
-Changes since 20030710:
+XXX: Changes since 20030712a:
+- Bugfix: fix double free() in filter handling. Reported by Robert J. Clark.
+- Bugfix: fix some memory leaks.
+
+20030712a: Changes since 20030710a:
 - Bugfix: Filtering by regexp was broken (inverse logic) unless
   DEBUG_FILTER was set (debugmode included the 32 flag).
+  Regression introduced into 20030706a.
 
 Changes since 20030706:
 - Bugfix: LISTGROUP bugfixed, LISTGROUP - XOVER now works; no longer
Index: configutil.c
===================================================================
RCS file: /var/CVS/leafnode-2/configutil.c,v
retrieving revision 1.40
retrieving revision 1.41
diff -u -r1.40 -r1.41
--- configutil.c	6 Jul 2003 21:21:13 -0000	1.40
+++ configutil.c	13 Jul 2003 00:58:25 -0000	1.41
@@ -526,6 +526,8 @@
 	    free(p->username);
 	if (p->password)
 	    free(p->password);
+	if (p->group_pcre)
+	    free(p->group_pcre);
 	free(p);
 	p = t;
     }
Index: fetchnews.c
===================================================================
RCS file: /var/CVS/leafnode-2/fetchnews.c,v
retrieving revision 1.117
retrieving revision 1.118
diff -u -r1.117 -r1.118
--- fetchnews.c	21 Jun 2003 14:40:11 -0000	1.117
+++ fetchnews.c	13 Jul 2003 00:58:37 -0000	1.118
@@ -1439,8 +1439,8 @@
 	    if (e < 0)
 		ln_log(LNLOG_SERR, LNLOG_CGROUP, "cannot touch %s: %m", mastr_str(s));
 	}
-	mastr_delete(s);
     }
+    mastr_delete(s);
 }
 
 /* post article in open file f, return FALSE if problem, return TRUE if ok */
Index: filterutil.c
===================================================================
RCS file: /var/CVS/leafnode-2/filterutil.c,v
retrieving revision 1.28
retrieving revision 1.29
diff -u -r1.28 -r1.29
--- filterutil.c	12 Jul 2003 23:32:01 -0000	1.28
+++ filterutil.c	13 Jul 2003 00:55:47 -0000	1.29
@@ -246,7 +246,6 @@
 {
     FILE *ff;
     char *l;
-    pcre *ng = NULL;
     char *param, *value, *ngt = NULL;
     struct filterlist *f;
     int rv = TRUE, invertngs = 0;
@@ -293,13 +292,11 @@
 		    invertngs = 1;
 		} else
 		    invertngs = 0;
-		ng = ln_pcre_compile(value, 0, NULL, filterfilename, line);
-		if (ng) state = RF_WANTPAT;
-		else rv = FALSE;
+		state = RF_WANTPAT;
 	    } else if ((state == RF_WANTPAT || state == RF_WANTNGORPAT) &&
 		    !strcasecmp("pattern", param)) {
-		pcre *re;
-		if (!ng) {
+		pcre *re, *ngp;
+		if (!ngt || !(ngp = ln_pcre_compile(ngt, 0, NULL, filterfilename, line))) {
 		    ln_log(LNLOG_SNOTICE, LNLOG_CTOP,
 			    "No newsgroup for pattern = %s (line %lu) found",
 			    value, line);
@@ -316,7 +313,7 @@
 			filterfilename, line);
 		if (re) {
 		    f = newfilter();
-		    insertfilter(f, ng, critstrdup(ngt, "readfilter"), invertngs);
+		    insertfilter(f, ngp, critstrdup(ngt, "readfilter"), invertngs);
 		    (f->entry)->expr = re;
 		    (f->entry)->cleartext = critstrdup(value, "readfilter");
 		} else {
@@ -329,8 +326,13 @@
 			(!strcasecmp("maxlines", param)) ||
 			(!strcasecmp("maxbytes", param)) ||
 			(!strcasecmp("maxcrosspost", param)))) {
+		pcre *ngp;
 		f = newfilter();
-		insertfilter(f, ng, critstrdup(ngt, "readfilter"), invertngs);
+		if (!(ngp = ln_pcre_compile(ngt, 0, NULL, filterfilename, line))) {
+		    rv = FALSE;
+		    continue;
+		}
+		insertfilter(f, ngp, critstrdup(ngt, "readfilter"), invertngs);
 		(f->entry)->cleartext = critstrdup(param, "readfilter");
 		(f->entry)->limit = (int)strtol(value, NULL, 10);
 		state = RF_WANTACTION;
@@ -361,10 +363,10 @@
 	}
     } /* while */
     (void)fclose(ff);
-    if (ng)
-	free(ng);
     free(param);
     free(value);
+    if (ngt)
+	free(ngt);
     if (state != RF_WANTNG && state != RF_WANTNGORPAT) {
 	ln_log(LNLOG_SERR, LNLOG_CTOP,
 	       "%s:%lu: premature end of file, expected %s",


-- 
Matthias Andree

-- 
leafnode-list@xxxxxxxxxxxxxxxxxxxxxxxxxxxx -- mailing list for leafnode
To unsubscribe, send mail with "unsubscribe" in the subject to the list