Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 1 Jul 2003 10:55:29 -0400 
From:      Don Bowman <don@sandvine.com>
To:        "'freebsd-net@freebsd.org'" <freebsd-net@freebsd.org>
Subject:   RE: using memory after freed in tcp_syncache (syncache_timer()) w ith ipfw: patch attached
Message-ID:  <FE045D4D9F7AED4CBFF1B3B813C8533702741C16@mail.sandvine.com>

next in thread | raw e-mail | index | archive | help
From: Don Bowman [mailto:don@sandvine.com]
> 
> 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.
> 

There was an error in the previous patch.

Index: tcp_syncache.c
===================================================================
RCS file: /usr/cvs/src/sys/netinet/tcp_syncache.c,v
retrieving revision 1.5.2.8.1000.3
diff -U5 -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 14:32:29 -0000
@@ -83,16 +83,18 @@
 #endif /*IPSEC*/
 
 #include <machine/in_cksum.h>
 #include <vm/vm_zone.h>
 
+static int syncache_delete_flag;
 static int tcp_syncookies = 1;
 SYSCTL_INT(_net_inet_tcp, OID_AUTO, syncookies, CTLFLAG_RW,
     &tcp_syncookies, 0, 
     "Use TCP SYN cookies if the syncache overflows");
 
 static void	 syncache_drop(struct syncache *, struct syncache_head *);
+static void	 syncache_delete(struct syncache *, struct syncache_head *);
 static void	 syncache_free(struct syncache *);
 static void	 syncache_insert(struct syncache *, struct syncache_head *);
 struct syncache *syncache_lookup(struct in_conninfo *, struct syncache_head
**);
 static int	 syncache_respond(struct syncache *, struct mbuf *);
 static struct 	 socket *syncache_socket(struct syncache *, struct socket
*);
@@ -125,10 +127,11 @@
 	u_int	next_reseed;
 	TAILQ_HEAD(, syncache) timerq[SYNCACHE_MAXREXMTS + 1];
 	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");
 
 SYSCTL_INT(_net_inet_tcp_syncache, OID_AUTO, bucketlimit, CTLFLAG_RD,
      &tcp_syncache.bucket_limit, 0, "Per-bucket hash limit for syncache");
@@ -202,10 +205,13 @@
 			rtrequest(RTM_DELETE, rt_key(rt),
 			    rt->rt_gateway, rt_mask(rt),
 			    rt->rt_flags, NULL);
 		RTFREE(rt);
 	}
+#if defined(DIAGNOSTIC)
+	memset(sc, 0xee, sizeof(struct syncache));
+#endif
 	zfree(tcp_syncache.zone, sc);
 }
 
 void
 syncache_init(void)
@@ -256,10 +262,12 @@
 	 * older one.
 	 */
 	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
 syncache_insert(sc, sch)
 	struct syncache *sc;
@@ -312,12 +320,28 @@
 static void
 syncache_drop(sc, sch)
 	struct syncache *sc;
 	struct syncache_head *sch;
 {
+	if ((sc->sc_flags & SCF_DELETE) == 0) {
+		sc->sc_flags |= SCF_DELETE;
+		syncache_delete_flag = 1;
+		TAILQ_INSERT_TAIL(&sc_delete_list, sc, sc_delete);
+	}
+}
+
+static void
+syncache_delete(sc, sch)
+	struct syncache *sc;
+	struct syncache_head *sch;
+{
 	int s;
 
+	if ((sc->sc_flags & SCF_DELETE) == 0) {
+	    printf("ERROR ERROR ERROR: SCF_DELETE == 0\n");
+	    return;
+	}
 	if (sch == NULL) {
 #ifdef INET6
 		if (sc->sc_inc.inc_isipv6) {
 			sch = &tcp_syncache.hashbase[
 			    SYNCACHE_HASH6(&sc->sc_inc,
tcp_syncache.hashmask)];
@@ -329,10 +353,12 @@
 		}
 	}
 
 	s = splnet();
 
+	TAILQ_REMOVE(&sc_delete_list, sc, sc_delete);
+
 	TAILQ_REMOVE(&sch->sch_bucket, sc, sc_hash);
 	sch->sch_length--;
 	tcp_syncache.cache_count--;
 
 	TAILQ_REMOVE(&tcp_syncache.timerq[sc->sc_rxtslot], sc, sc_timerq);
@@ -357,10 +383,12 @@
 	int s;
 
 	s = splnet();
         if (callout_pending(&tcp_syncache.tt_timerq[slot]) ||
             !callout_active(&tcp_syncache.tt_timerq[slot])) {
+		if (syncache_delete_flag)
+			goto delete_cleanup;
                 splx(s);
                 return;
         }
         callout_deactivate(&tcp_syncache.tt_timerq[slot]);
 
@@ -390,10 +418,21 @@
 		SYNCACHE_TIMEOUT(sc, slot + 1);
 	}
 	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_delete(sc, NULL); 
+		sc = nsc;
+	}
+	TAILQ_INIT(&sc_delete_list);
+	syncache_delete_flag = 0;
+
 	splx(s);
 }
 
 /*
  * Find an entry in the syncache.
@@ -1333,10 +1372,11 @@
 	data = data >> SYNCOOKIE_WNDBITS;
 
 	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
 	 */
 	sc->sc_ipopts = NULL;
Index: tcp_var.h
===================================================================
RCS file: /usr/cvs/src/sys/netinet/tcp_var.h,v
retrieving revision 1.56.2.12
diff -U5 -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
@@ -222,12 +222,14 @@
 #define SCF_WINSCALE	0x02			/* negotiated window scaling
*/
 #define SCF_TIMESTAMP	0x04			/* negotiated timestamps */
 #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 {
 	TAILQ_HEAD(, syncache)	sch_bucket;
 	u_int		sch_length;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?FE045D4D9F7AED4CBFF1B3B813C8533702741C16>