From owner-freebsd-net@FreeBSD.ORG Tue Jul 1 07:55:39 2003 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 6D66F37B401 for ; Tue, 1 Jul 2003 07:55:39 -0700 (PDT) Received: from mail.sandvine.com (sandvine.com [199.243.201.138]) by mx1.FreeBSD.org (Postfix) with ESMTP id 960A944025 for ; Tue, 1 Jul 2003 07:55:38 -0700 (PDT) (envelope-from don@sandvine.com) Received: by mail.sandvine.com with Internet Mail Service (5.5.2653.19) id ; Tue, 1 Jul 2003 10:55:37 -0400 Message-ID: From: Don Bowman To: "'freebsd-net@freebsd.org'" Date: Tue, 1 Jul 2003 10:55:29 -0400 MIME-Version: 1.0 X-Mailer: Internet Mail Service (5.5.2653.19) Content-Type: text/plain; charset="iso-8859-1" Subject: RE: using memory after freed in tcp_syncache (syncache_timer()) w ith ipfw: patch attached X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Jul 2003 14:55:39 -0000 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 #include +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;