[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freebsd-net
Subject:    Re: panic with tcp timers
From:       "K. Macy" <kmacy () freebsd ! org>
Date:       2016-06-28 22:51:57
Message-ID: CAHM0Q_OEYhAqqgPj3Zn9z3KNn7c-=B8CLFWR=S7_fU2ePknt7A () mail ! gmail ! com
[Download RAW message or body]

On Tue, Jun 28, 2016 at 10:51 AM, Matthew Macy <mmacy@nextbsd.org> wrote:
> You guys should really look at Samy Bahra's epoch based reclamation. I solved a \
> similar problem in drm/linuxkpi using it.

The point being that this is a bug in the TCP life cycle handling
_not_ in callouts. Churning the callout interface is not the best /
only solution.
-M


> 
> ---- On Tue, 28 Jun 2016 02:58:56 -0700 Julien Charbon <jch@freebsd.org> wrote ----
> > 
> > Hi Randall,
> > 
> > On 6/25/16 4:41 PM, Randall Stewart via freebsd-net wrote:
> > > Ok
> > > 
> > > Lets try this again with my source changed to my @freebsd.net :-)
> > > 
> > > Now I am also attaching a patch for you Gleb, this will take some poking to
> > > get in to your NF-head since it incorporates some changes we made earlier.
> > > 
> > > I think this will fix the problem.. i.e. dealing with two locks in the callout \
> > > system (which it was never meant to have done)..
> > > 
> > > Note we probably can move the code to use the callout lock init now.. but lets \
> > > see if this works on your setup on c096 and if so we can think about doing \
> > > that.
> > 
> > Thanks for proposing a patch.  I believe your patch will work with
> > callout lock init, but not without:  You still have a use-after-free
> > issue on the tcpcb without callout lock init.
> > 
> > The case being subtle as usual, let me try to describe that could happen:
> > 
> > With your patch we have:
> > 
> > void
> > tcp_timer_keep(void *xtp)
> > {
> > struct tcpcb *tp = xtp;
> > struct tcptemp *t_template;
> > struct inpcb *inp;
> > CURVNET_SET(tp->t_vnet);
> > #ifdef TCPDEBUG
> > int ostate;
> > 
> > ostate = tp->t_state;
> > #endif
> > inp = tp->t_inpcb;
> > KASSERT(inp != NULL, ("%s: tp %p tp->t_inpcb == NULL", __func__,
> > tp));
> > INP_WLOCK(inp);
> > if (callout_pending(&tp->t_timers->tt_keep) ### Use after free
> > of tp here
> > !callout_active(&tp->t_timers->tt_keep)) {
> > INP_WUNLOCK(inp);
> > CURVNET_RESTORE();
> > return;
> > }
> > ...
> > 
> > The use-after-free scenario:
> > 
> > [CPU 1] the callout fires, tcp_timer_keep entered
> > [CPU 1] blocks on INP_WLOCK(inp);
> > [CPU 2] schedules tcp_timer_keep with callout_reset()
> > [CPU 2] tcp_discardcb called
> > [CPU 2] tcp_timer_keep callout successfully canceled
> > [CPU 2] tcpcb freed
> > [CPU 1] unblocks, the tcpcb is used
> > 
> > Then the tcpcb will used just after being freed...  Might also crash or
> > not depending in the case.
> > 
> > Extra notes:
> > 
> > o The invariant I see here is:  The "callout successfully canceled"
> > step should never happen when "the callout is currently being executed".
> > 
> > o Solutions I see to enforce this invariant:
> > 
> > - First solution:  Use callout lock init with inp lock, your patch
> > seems to permit that now.
> > 
> > - Second solution:  Change callout_async_drain() behavior:  It can
> > return 0 (fail) when the callout is currently being executed (no matter
> > what).
> > 
> > - Third solution:  Don't trust callout_async_drain(callout) return
> > value of 1 (success) if the previous call of callout_reset(callout)
> > returned 0 (fail).  That was the exact purpose of r284261 change, but
> > this solution is also step backward in modernization of TCP
> > timers/callout...
> > 
> > https://svnweb.freebsd.org/base/stable/10/sys/netinet/tcp_timer.c?r1=284261&r2=284260&pathrev=284261
> >  
> > Hopefully my description is clear enough...
> > 
> > --
> > Julien
> > 
> > 
> 
> _______________________________________________
> freebsd-current@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"
_______________________________________________
freebsd-net@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"


[prev in list] [next in list] [prev in thread] [next in thread]