From owner-freebsd-net@freebsd.org Tue Jun 28 22:51:59 2016 Return-Path: Delivered-To: freebsd-net@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 95232B86B0D for ; Tue, 28 Jun 2016 22:51:59 +0000 (UTC) (envelope-from kmacybsd@gmail.com) Received: from mailman.ysv.freebsd.org (unknown [127.0.1.3]) by mx1.freebsd.org (Postfix) with ESMTP id 7199225C1 for ; Tue, 28 Jun 2016 22:51:59 +0000 (UTC) (envelope-from kmacybsd@gmail.com) Received: by mailman.ysv.freebsd.org (Postfix) id 69FB8B86B0B; Tue, 28 Jun 2016 22:51:59 +0000 (UTC) Delivered-To: net@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 697E2B86B09; Tue, 28 Jun 2016 22:51:59 +0000 (UTC) (envelope-from kmacybsd@gmail.com) Received: from mail-io0-x236.google.com (mail-io0-x236.google.com [IPv6:2607:f8b0:4001:c06::236]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 2CA0F25BD; Tue, 28 Jun 2016 22:51:59 +0000 (UTC) (envelope-from kmacybsd@gmail.com) Received: by mail-io0-x236.google.com with SMTP id g13so30415030ioj.1; Tue, 28 Jun 2016 15:51:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=1U9wqxNBLZh/U1HLxO7LU4TTr/tE6NgZK0hb0cSEdDY=; b=iz205537VSNKyLcgIqKfIVSj+iMMJqyPwtBSfheBzjSl1ySMt1Wo2wNgd0N7H0eIfC 4HqQdBuqeM6gs8tI82hZGLgjQHHd2lSV284vp6sWmw+kCobYqJkLBLhmMoJL/kx7BCF5 izFI6sc7MKZ44tuEKydnlpg6WQBztugypfT8v7u0lsOBZdy8itLZeuql5WtiDCQKViM9 bG+euTckX+nF8IpzpmzDkb8yiVyWQm3LqpezarXJ91en5GElHS1vy5EtdLkZlI86Z3fJ qg48slAvvjAFwKxpgLa5876M9yLCgKomUqTd407hVSLcelRUWAQ9Gg+4znExYSocua3s Gmpg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=1U9wqxNBLZh/U1HLxO7LU4TTr/tE6NgZK0hb0cSEdDY=; b=e/HnNNwYmDU3QFYsWgc/QX0RNuXoyZlY7wFh95v15OBBofF95RcVU57skUJsPuMjAh Siy8ZaZckQXrk45aAX1EN8XteLvr6Dov8I3FYhqmSPsHscklp+K0psg0sTbu/liNEcG/ 5DFgfsGlWDxJPi8j4ke06qwpHzk/9rXHL+DetHMlbdmMdxxBoTzsthIhMcXwfW/xQa// WhtN0jH3GOX0YyMo6AruO8+in7/iAp6Nk+xKN3GX2iuirqaDx5bpWBoPRzuFGaiQCqu7 m266TwlbugIl7aRQT/1NxukTK1rSygYlRrW8FQhTMdAOhb3BZvT/oad93q62jjj8jvaO 3x3Q== X-Gm-Message-State: ALyK8tI6pSUZMHPp4gVx8UOaK6WdnY+Df0AawcRHmguzl8OU5hsoxpChryXvtXihPZLnr6LfuJm4PRGCy6n2Zg== X-Received: by 10.107.162.211 with SMTP id l202mr4847414ioe.138.1467154318474; Tue, 28 Jun 2016 15:51:58 -0700 (PDT) MIME-Version: 1.0 Sender: kmacybsd@gmail.com Received: by 10.107.134.218 with HTTP; Tue, 28 Jun 2016 15:51:57 -0700 (PDT) In-Reply-To: <15598235139.12175f84421756.2471769249719458878@nextbsd.org> References: <20160617045319.GE1076@FreeBSD.org> <1f28844b-b4ea-b544-3892-811f2be327b9@freebsd.org> <20160620073917.GI1076@FreeBSD.org> <1d18d0e2-3e42-cb26-928c-2989d0751884@freebsd.org> <20160620095822.GJ1076@FreeBSD.org> <74bb31b7-a9f5-3d0c-eea0-681872e6f09b@freebsd.org> <18D94615-810E-4E79-A889-4B0CC70F9E45@netflix.com> <6E52CA6A-2153-4EF9-A3E1-97CB0D07EB28@freebsd.org> <15598235139.12175f84421756.2471769249719458878@nextbsd.org> From: "K. Macy" Date: Tue, 28 Jun 2016 15:51:57 -0700 X-Google-Sender-Auth: DZLaI5R3EaUjJmPRLjgY8ddZcJQ Message-ID: Subject: Re: panic with tcp timers To: Matthew Macy Cc: Julien Charbon , Randall Stewart , "current@freebsd.org" , Hans Petter Selasky , "freebsd-net@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.22 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, 28 Jun 2016 22:51:59 -0000 On Tue, Jun 28, 2016 at 10:51 AM, Matthew Macy 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 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"