netdev - Re: [PATCH net 0/2] ppp: fix deadlock upon recursive xmit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 28 Aug 2016 20:18:24 +0800
From:   Feng Gao <gfree.wind@...il.com>
To:     Guillaume Nault <g.nault@...halink.fr>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Paul Mackerras <paulus@...ba.org>
Subject: Re: [PATCH net 0/2] ppp: fix deadlock upon recursive xmit

On Sun, Aug 28, 2016 at 4:20 AM, Guillaume Nault <g.nault@...halink.fr> wrote:
> This series fixes the issue reported by Feng where packets looping
> through a ppp device makes the module deadlock:
> https://marc.info/?l=linux-netdev&m=147134567319038&w=2
>
> The problem can occur on virtual interfaces (e.g. PPP over L2TP, or
> PPPoE on vxlan devices), when a PPP packet is routed back to the PPP
> interface.
>
> PPP's xmit path isn't reentrant, so patch #1 uses a per-cpu variable
> to detect and break recursion. Patch #2 sets the NETIF_F_LLTX flag to
> avoid lock inversion issues between ppp and txqueue locks.
>
> There are multiple entry points to the PPP xmit path. This series has
> been tested with lockdep and should address recursion issues no matter
> how the packet entered the path.
>
>
> A similar issue in L2TP is not covered by this series:
> l2tp_xmit_skb() also isn't reentrant, and it can be called as part of
> PPP's xmit path (pppol2tp_xmit()), or directly from the L2TP socket
> (l2tp_ppp_sendmsg()). If a packet is sent by l2tp_ppp_sendmsg() and
> routed to the parent PPP interface, then it's going to hit
> l2tp_xmit_skb() again.
>
> Breaking recursion as done in ppp_generic is not enough, because we'd
> still have a lock inversion issue (locking in l2tp_xmit_skb() can
> happen before or after locking in ppp_generic). The best approach would
> be to use the ip_tunnel functions and remove the socket locking in
> l2tp_xmit_skb(). But that'd be something for net-next.
>
>
> BTW, I hope the commit messages aren't too long. Just let me know if I
> should trim something.
>
>
> Guillaume Nault (2):
>   ppp: avoid dealock on recursive xmit
>   ppp: declare PPP devices as LLTX
>
>  drivers/net/ppp/ppp_generic.c | 54 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 42 insertions(+), 12 deletions(-)
>
> --
> 2.9.3
>

I am learning your codes. It is better than my solution :))

Best Regards
Feng