using memory after freed in tcp_syncache (syncache_timer()) w ith ipfw: patch attached

[Don Bowman]

Synopsis: under some ipfw conditions, tcp_syncache has
syncache_respond() call ip_output call ip_input call syncache_drop(),
which drops the 'syncache' that is being worked on, or corrupts
the list, etc. This is typically seen from syncache_timer or
syncache_add.

I've attached a patch that I believe corrects this problem.
I'm observing it on 4.7, but I believe it equally affects RELENG_4
and CURRENT.

This seems to make the problem I was seeing go away. I'm
currently running with 2K syn/second through the original condition,
will let it go overnight like that. I think that will flush
out if i've introduced a leak or other crash.

Can someone who knows this code perhaps critique what I've done?

Essentially I have made syncache_drop() instead defer the delete
onto a different list. In the timer, I delete the syncache entries
from the delete list. This costs some performance and memory, but
was the best way I could come up with.

 --don

Index: tcp_syncache.c
===================================================================
RCS file: /usr/cvs/src/sys/netinet/tcp_syncache.c,v
retrieving revision 1.5.2.8.1000.3
diff -U3 -r1.5.2.8.1000.3 tcp_syncache.c
--- tcp_syncache.c	4 Feb 2003 01:52:03 -0000	1.5.2.8.1000.3
+++ tcp_syncache.c	1 Jul 2003 03:05:22 -0000
@@ -85,6 +85,7 @@
 #include <machine/in_cksum.h>
 #include <vm/vm_zone.h>
 
+static int syncache_delete;
 static int tcp_syncookies = 1;
 SYSCTL_INT(_net_inet_tcp, OID_AUTO, syncookies, CTLFLAG_RW,
     &tcp_syncookies, 0, 
@@ -127,6 +128,7 @@
 	struct	callout tt_timerq[SYNCACHE_MAXREXMTS + 1];
 };
 static struct tcp_syncache tcp_syncache;
+static TAILQ_HEAD(syncache_delete_list, syncache)	sc_delete_list;
 
 SYSCTL_NODE(_net_inet_tcp, OID_AUTO, syncache, CTLFLAG_RW, 0, "TCP SYN
cache");
 
@@ -204,6 +206,9 @@
 			    rt->rt_flags, NULL);
 		RTFREE(rt);
 	}
+#if defined(DIAGNOSTIC)
+	memset(sc, 0xee, sizeof(struct syncache));
+#endif
 	zfree(tcp_syncache.zone, sc);
 }
 
@@ -258,6 +263,8 @@
 	tcp_syncache.cache_limit -= 1;
 	tcp_syncache.zone = zinit("syncache", sizeof(struct syncache),
 	    tcp_syncache.cache_limit, ZONE_INTERRUPT, 0);
+
+	TAILQ_INIT(&sc_delete_list);
 }
 
 static void
@@ -331,6 +338,18 @@
 
 	s = splnet();
 
+	if ((sc->sc_flags & SCF_DELETE) == 0) {
+		sc->sc_flags |= SCF_DELETE;
+		syncache_delete = 1;
+		TAILQ_INSERT_TAIL(&sc_delete_list, sc, sc_delete);
+
+		splx(s);
+		return;
+	}
+	if (sc->sc_delete.tqe_next || sc->sc_delete.tqe_prev) {
+		TAILQ_REMOVE(&sc_delete_list, sc, sc_delete);
+	}
+
 	TAILQ_REMOVE(&sch->sch_bucket, sc, sc_hash);
 	sch->sch_length--;
 	tcp_syncache.cache_count--;
@@ -359,6 +378,8 @@
 	s = splnet();
         if (callout_pending(&tcp_syncache.tt_timerq[slot]) ||
             !callout_active(&tcp_syncache.tt_timerq[slot])) {
+		if (syncache_delete)
+			goto delete_cleanup;
                 splx(s);
                 return;
         }
@@ -392,6 +413,17 @@
 	if (nsc != NULL)
 		callout_reset(&tcp_syncache.tt_timerq[slot],
 		    nsc->sc_rxttime - ticks, syncache_timer, (void
*)(slot));
+
+delete_cleanup:
+	sc = TAILQ_FIRST(&sc_delete_list);
+	while (sc != NULL) {
+		nsc = TAILQ_NEXT(sc, sc_delete);
+		syncache_drop(sc, NULL); 
+		sc = nsc;
+	}
+	TAILQ_INIT(&sc_delete_list);
+	syncache_delete = 0;
+
 	splx(s);
 }
 
@@ -1335,6 +1367,7 @@
 	sc = zalloc(tcp_syncache.zone);
 	if (sc == NULL)
 		return (NULL);
+	bzero(sc, sizeof(*sc));
 	/*
 	 * Fill in the syncache values.
 	 * XXX duplicate code from syncache_add
Index: tcp_var.h
===================================================================
RCS file: /usr/cvs/src/sys/netinet/tcp_var.h,v
retrieving revision 1.56.2.12
diff -U3 -r1.56.2.12 tcp_var.h
--- tcp_var.h	24 Aug 2002 18:40:26 -0000	1.56.2.12
+++ tcp_var.h	1 Jul 2003 02:33:57 -0000
@@ -224,8 +224,10 @@
 #define SCF_CC		0x08			/* negotiated CC */
 #define SCF_UNREACH	0x10			/* icmp unreachable received
*/
 #define SCF_KEEPROUTE	0x20			/* keep cloned route */
+#define SCF_DELETE	0x40			/* I'm being deleted */
 	TAILQ_ENTRY(syncache)	sc_hash;
 	TAILQ_ENTRY(syncache)	sc_timerq;
+	TAILQ_ENTRY(syncache)	sc_delete;
 };
 
 struct syncache_head {