From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from shards.monkeyblade.net ([184.105.139.130]:57464 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751505AbeCVQgP (ORCPT ); Thu, 22 Mar 2018 12:36:15 -0400 Date: Thu, 22 Mar 2018 12:36:13 -0400 (EDT) Message-Id: <20180322.123613.1521600246177555800.davem@davemloft.net> To: g.nault@alphalink.fr Cc: netdev@vger.kernel.org, paulus@samba.org, xuheng333@zoho.com, xeb@mail.ru, stephen@networkplumber.org Subject: Re: [PATCH net] ppp: avoid loop in xmit recursion detection code From: David Miller In-Reply-To: References: Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: From: Guillaume Nault Date: Tue, 20 Mar 2018 16:49:26 +0100 > We already detect situations where a PPP channel sends packets back to > its upper PPP device. While this is enough to avoid deadlocking on xmit > locks, this doesn't prevent packets from looping between the channel > and the unit. > > The problem is that ppp_start_xmit() enqueues packets in ppp->file.xq > before checking for xmit recursion. Therefore, __ppp_xmit_process() > might dequeue a packet from ppp->file.xq and send it on the channel > which, in turn, loops it back on the unit. Then ppp_start_xmit() > queues the packet back to ppp->file.xq and __ppp_xmit_process() picks > it up and sends it again through the channel. Therefore, the packet > will loop between __ppp_xmit_process() and ppp_start_xmit() until some > other part of the xmit path drops it. > > For L2TP, we rapidly fill the skb's headroom and pppol2tp_xmit() drops > the packet after a few iterations. But PPTP reallocates the headroom > if necessary, letting the loop run and exhaust the machine resources > (as reported in https://bugzilla.kernel.org/show_bug.cgi?id=199109). > > Fix this by letting __ppp_xmit_process() enqueue the skb to > ppp->file.xq, so that we can check for recursion before adding it to > the queue. Now ppp_xmit_process() can drop the packet when recursion is > detected. > > __ppp_channel_push() is a bit special. It calls __ppp_xmit_process() > without having any actual packet to send. This is used by > ppp_output_wakeup() to re-enable transmission on the parent unit (for > implementations like ppp_async.c, where the .start_xmit() function > might not consume the skb, leaving it in ppp->xmit_pending and > disabling transmission). > Therefore, __ppp_xmit_process() needs to handle the case where skb is > NULL, dequeuing as many packets as possible from ppp->file.xq. > > Reported-by: xu heng > Fixes: 55454a565836 ("ppp: avoid dealock on recursive xmit") > Signed-off-by: Guillaume Nault Applied and queued up for -stable, thank you.