From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guillaume Nault Subject: Re: [PATCH net] l2tp: fix refcount leakage on PPPoL2TP sockets Date: Tue, 5 Jun 2018 18:37:32 +0200 Message-ID: <20180605163732.GA1458@alphalink.fr> References: <6021bb159fc069256f8947622e4b180d234fea0a.1528129907.git.g.nault@alphalink.fr> <20180605.094124.1023096251110931871.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, jchapman@katalix.com To: David Miller Return-path: Received: from zimbra.alphalink.fr ([217.15.80.77]:46164 "EHLO zimbra.alphalink.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751688AbeFEQhf (ORCPT ); Tue, 5 Jun 2018 12:37:35 -0400 Content-Disposition: inline In-Reply-To: <20180605.094124.1023096251110931871.davem@davemloft.net> Sender: netdev-owner@vger.kernel.org List-ID: On Tue, Jun 05, 2018 at 09:41:24AM -0400, David Miller wrote: > From: Guillaume Nault > Date: Mon, 4 Jun 2018 18:52:19 +0200 > > > Commit d02ba2a6110c ("l2tp: fix race in pppol2tp_release with session > > object destroy") tried to fix a race condition where a PPPoL2TP socket > > would disappear while the L2TP session was still using it. However, it > > missed the root issue which is that an L2TP session may accept to be > > reconnected if its associated socket has entered the release process. > > > > The tentative fix makes the session hold the socket it is connected to. > > That saves the kernel from crashing, but introduces refcount leakage, > > preventing the socket from completing the release process. Once stalled, > > everything the socket depends on can't be released anymore, including > > the L2TP session and the l2tp_ppp module. > ... > > So it all boils down to pppol2tp_connect() failing to realise that the > > session has already been connected. This patch drops the unneeded extra > > reference counting (mostly reverting d02ba2a6110c) and checks that > > neither ->sk nor ->__sk is set before allowing a session to be > > connected. > > > > Fixes: d02ba2a6110c ("l2tp: fix race in pppol2tp_release with session object destroy") > > Signed-off-by: Guillaume Nault > > So much fidgeting around in this area over the past year or two :-) > Putting L2TP into production without adding custom workarounds has been such a long journey, but we're almost there :-) > Applied and queued up for -stable, thanks for fixing this. > I still have a handful of issues to fix, though.