Date: Mon, 2 Oct 2017 13:16:42 +0200 From: Julien Charbon <jch@freebsd.org> To: gljennjohn@gmail.com Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r324179 - head/sys/netinet Message-ID: <5c69c015-f3ae-954c-0f54-bf487eb6cab4@freebsd.org> In-Reply-To: <20171002131218.3160093f@ernst.home> References: <201710012120.v91LKSH1050145@repo.freebsd.org> <20171002131218.3160093f@ernst.home>
next in thread | previous in thread | raw e-mail | index | archive | help
On 10/2/17 1:12 PM, Gary Jennejohn wrote: > On Sun, 1 Oct 2017 21:20:28 +0000 (UTC) > Julien Charbon <jch@FreeBSD.org> wrote: > >> Author: jch >> Date: Sun Oct 1 21:20:28 2017 >> New Revision: 324179 >> URL: https://svnweb.freebsd.org/changeset/base/324179 >> >> Log: >> Fix an infinite loop in tcp_tw_2msl_scan() when an INP_TIMEWAIT inp has >> been destroyed before its tcptw with INVARIANTS undefined. >> >> This is a symmetric change of r307551: >> >> A INP_TIMEWAIT inp should not be destroyed before its tcptw, and INVARIANTS >> will catch this case. If INVARIANTS is undefined it will emit a log(LOG_ERR) >> and avoid a hard to debug infinite loop in tcp_tw_2msl_scan(). >> >> Reported by: Ben Rubson, hselasky >> Submitted by: hselasky >> Tested by: Ben Rubson, jch >> MFC after: 1 week >> Sponsored by: Verisign, inc >> Differential Revision: https://reviews.freebsd.org/D12267 >> >> Modified: >> head/sys/netinet/tcp_timewait.c >> >> Modified: head/sys/netinet/tcp_timewait.c >> ============================================================================== >> --- head/sys/netinet/tcp_timewait.c Sun Oct 1 20:12:30 2017 (r324178) >> +++ head/sys/netinet/tcp_timewait.c Sun Oct 1 21:20:28 2017 (r324179) >> @@ -709,10 +709,29 @@ tcp_tw_2msl_scan(int reuse) >> INP_WLOCK(inp); >> tw = intotw(inp); >> if (in_pcbrele_wlocked(inp)) { >> - KASSERT(tw == NULL, ("%s: held last inp " >> - "reference but tw not NULL", __func__)); >> - INP_INFO_RUNLOCK(&V_tcbinfo); >> - continue; >> + if (__predict_true(tw == NULL)) { >> + INP_INFO_RUNLOCK(&V_tcbinfo); >> + continue; >> + } else { >> + /* This should not happen as in TIMEWAIT >> + * state the inp should not be destroyed >> + * before its tcptw. If INVARIANTS is >> + * defined panic. >> + */ >> +#ifdef INVARIANTS >> + panic("%s: Panic before an infinite " >> + "loop: INP_TIMEWAIT && (INP_FREED " >> + "|| inp last reference) && tw != " >> + "NULL", __func__); >> +#else >> + log(LOG_ERR, "%s: Avoid an infinite " >> + "loop: INP_TIMEWAIT && (INP_FREED " >> + "|| inp last reference) && tw != " >> + "NULL", __func__); >> +#endif >> + INP_INFO_RUNLOCK(&V_tcbinfo); >> + break; >> + } >> } >> >> if (tw == NULL) { >> > > This file needs to include sys/syslog.h, otherwise LOG_ERR is > not defined and the kernel build fails when INVARIANTS is not > defined in the kernel config file. You are correct and thanks for the report. I had sys/syslog.h already included as part of unrelated changes, then I forgot to push it: Fixed in r324193: https://svnweb.freebsd.org/base?view=revision&revision=324193 https://svnweb.freebsd.org/base/head/sys/netinet/tcp_timewait.c?r1=324193&r2=324192&pathrev=324193 -- Julien
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5c69c015-f3ae-954c-0f54-bf487eb6cab4>