From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [PATCH net] l2tp: fix refcount leakage on PPPoL2TP sockets Date: Tue, 05 Jun 2018 09:41:24 -0400 (EDT) Message-ID: <20180605.094124.1023096251110931871.davem@davemloft.net> References: <6021bb159fc069256f8947622e4b180d234fea0a.1528129907.git.g.nault@alphalink.fr> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, jchapman@katalix.com To: g.nault@alphalink.fr Return-path: Received: from shards.monkeyblade.net ([184.105.139.130]:35978 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751750AbeFENl2 (ORCPT ); Tue, 5 Jun 2018 09:41:28 -0400 In-Reply-To: <6021bb159fc069256f8947622e4b180d234fea0a.1528129907.git.g.nault@alphalink.fr> Sender: netdev-owner@vger.kernel.org List-ID: 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 :-) Applied and queued up for -stable, thanks for fixing this.