1 files changed, 18 insertions, 18 deletions

diff --git a/fw/src/rules.c b/fw/src/rules.c
index c0df82d..55de9cb 100644
--- a/fw/src/rules.c
+++ b/fw/src/rules.c
@@ -30,7 +30,6 @@ Rule	*rules;
 void	(*rulechanged)(void);
 
 static Lock	rulelock;
-static Rule	*lastrule;
 static char	*rulefile;
 static char	*parseerr;
 static jmp_buf	parsejmp;
@@ -135,7 +134,7 @@ verbof(char *s)
  * a firewall gets to do.
  */
 static void
-addrule(Ndbtuple *t, int nr)
+addrule(Ndbtuple *t, int nr, Rule **head, Rule **tail)
 {
 	char *ip, *mask, *p, abuf[64];
 	Rule *r;
@@ -206,28 +205,34 @@ addrule(Ndbtuple *t, int nr)
 		r->anyip = 0;
 	}
 
-	if(lastrule == nil)
-		rules = r;
+	if(*head == nil)
+		*head = r;
 	else
-		lastrule->next = r;
-	lastrule = r;
+		(*tail)->next = r;
+	*tail = r;
 }
 
 /*
  * Parse without installing.  Returns the new list, or nil with *err set.
  * An empty file is a valid rule set: it denies everything.
  */
+/*
+ * Build the new list in locals.  Earlier this borrowed the globals and
+ * put them back afterwards, which meant that for the length of a reload
+ * the relay procs - which take rulelock, a lock this never held - were
+ * walking a list that was first empty and then half built.  Every packet
+ * in that window was judged against a partial rule set, and a parse
+ * failure freed nodes a relay might still have been holding.
+ */
 Rule*
 parserules(char *file, char **err)
 {
-	Rule *new, *save, *savelast;
+	Rule *head, *tail;
 	Ndbtuple *t;
 	Ndb *db;
 	int nr;
 
-	save = rules;
-	savelast = lastrule;
-	rules = lastrule = nil;
+	head = tail = nil;
 	rulefile = file;
 	parseerr = nil;
 
@@ -236,30 +241,25 @@ parserules(char *file, char **err)
 
 		snprint(eb, sizeof eb, "%s: %r", file);
 		*err = eb;
-		rules = save;
-		lastrule = savelast;
 		return nil;
 	}
 	parsing = 1;
 	if(setjmp(parsejmp) == 0){
 		for(nr = 1; (t = ndbparse(db)) != nil; nr++){
-			addrule(t, nr);
+			addrule(t, nr, &head, &tail);
 			ndbfree(t);
 		}
 	}
 	parsing = 0;
 	ndbclose(db);
 
-	new = rules;
-	rules = save;
-	lastrule = savelast;
 	if(parseerr != nil){
-		freerules(new);
+		freerules(head);
 		*err = parseerr;
 		return nil;
 	}
 	*err = nil;
-	return new;
+	return head;
 }
 
 void