1. 28 7月, 2018 4 次提交
  2. 27 7月, 2018 1 次提交
    • G
      l2tp: remove ->recv_payload_hook · 2b139e6b
      Guillaume Nault 提交于
      The tunnel reception hook is only used by l2tp_ppp for skipping PPP
      framing bytes. This is a session specific operation, but once a PPP
      session sets ->recv_payload_hook on its tunnel, all frames received by
      the tunnel will enter pppol2tp_recv_payload_hook(), including those
      targeted at Ethernet sessions (an L2TPv3 tunnel can multiplex PPP and
      Ethernet sessions).
      
      So this mechanism is wrong, and uselessly complex. Let's just move this
      functionality to the pppol2tp rx handler and drop ->recv_payload_hook.
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      2b139e6b
  3. 25 7月, 2018 1 次提交
  4. 07 7月, 2018 2 次提交
  5. 29 6月, 2018 1 次提交
    • L
      Revert changes to convert to ->poll_mask() and aio IOCB_CMD_POLL · a11e1d43
      Linus Torvalds 提交于
      The poll() changes were not well thought out, and completely
      unexplained.  They also caused a huge performance regression, because
      "->poll()" was no longer a trivial file operation that just called down
      to the underlying file operations, but instead did at least two indirect
      calls.
      
      Indirect calls are sadly slow now with the Spectre mitigation, but the
      performance problem could at least be largely mitigated by changing the
      "->get_poll_head()" operation to just have a per-file-descriptor pointer
      to the poll head instead.  That gets rid of one of the new indirections.
      
      But that doesn't fix the new complexity that is completely unwarranted
      for the regular case.  The (undocumented) reason for the poll() changes
      was some alleged AIO poll race fixing, but we don't make the common case
      slower and more complex for some uncommon special case, so this all
      really needs way more explanations and most likely a fundamental
      redesign.
      
      [ This revert is a revert of about 30 different commits, not reverted
        individually because that would just be unnecessarily messy  - Linus ]
      
      Cc: Al Viro <viro@zeniv.linux.org.uk>
      Cc: Christoph Hellwig <hch@lst.de>
      Signed-off-by: NLinus Torvalds <torvalds@linux-foundation.org>
      a11e1d43
  6. 28 6月, 2018 1 次提交
    • G
      l2tp: define helper for parsing struct sockaddr_pppol2tp* · a408194a
      Guillaume Nault 提交于
      'sockaddr_len' is checked against various values when entering
      pppol2tp_connect(), to verify its validity. It is used again later, to
      find out which sockaddr structure was passed from user space. This
      patch combines these two operations into one new function in order to
      simplify pppol2tp_connect().
      
      A new structure, l2tp_connect_info, is used to pass sockaddr data back
      to pppol2tp_connect(), to avoid passing too many parameters to
      l2tp_sockaddr_get_info(). Also, the first parameter is void* in order
      to avoid casting between all sockaddr_* structures manually.
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      a408194a
  7. 26 6月, 2018 7 次提交
  8. 16 6月, 2018 2 次提交
    • G
      l2tp: filter out non-PPP sessions in pppol2tp_tunnel_ioctl() · ecd012e4
      Guillaume Nault 提交于
      pppol2tp_tunnel_ioctl() can act on an L2TPv3 tunnel, in which case
      'session' may be an Ethernet pseudo-wire.
      
      However, pppol2tp_session_ioctl() expects a PPP pseudo-wire, as it
      assumes l2tp_session_priv() points to a pppol2tp_session structure. For
      an Ethernet pseudo-wire l2tp_session_priv() points to an l2tp_eth_sess
      structure instead, making pppol2tp_session_ioctl() access invalid
      memory.
      
      Fixes: d9e31d17 ("l2tp: Add L2TP ethernet pseudowire support")
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      ecd012e4
    • G
      l2tp: reject creation of non-PPP sessions on L2TPv2 tunnels · de9bada5
      Guillaume Nault 提交于
      The /proc/net/pppol2tp handlers (pppol2tp_seq_*()) iterate over all
      L2TPv2 tunnels, and rightfully expect that only PPP sessions can be
      found there. However, l2tp_netlink accepts creating Ethernet sessions
      regardless of the underlying tunnel version.
      
      This confuses pppol2tp_seq_session_show(), which expects that
      l2tp_session_priv() returns a pppol2tp_session structure. When the
      session is an Ethernet pseudo-wire, a struct l2tp_eth_sess is returned
      instead. This leads to invalid memory access when
      pppol2tp_session_get_sock() later tries to dereference ps->sk.
      
      Fixes: d9e31d17 ("l2tp: Add L2TP ethernet pseudowire support")
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      de9bada5
  9. 15 6月, 2018 4 次提交
  10. 05 6月, 2018 1 次提交
    • G
      l2tp: fix refcount leakage on PPPoL2TP sockets · 3d609342
      Guillaume Nault 提交于
      Commit d02ba2a6 ("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.
      
      The root issue is that, when releasing a connected PPPoL2TP socket, the
      session's ->sk pointer (RCU-protected) is reset to NULL and we have to
      wait for a grace period before destroying the socket. The socket drops
      the session in its ->sk_destruct callback function, so the session
      will exist until the last reference on the socket is dropped.
      Therefore, there is a time frame where pppol2tp_connect() may accept
      reconnecting a session, as it only checks ->sk to figure out if the
      session is connected. This time frame is shortened by the fact that
      pppol2tp_release() calls l2tp_session_delete(), making the session
      unreachable before resetting ->sk. However, pppol2tp_connect() may
      grab the session before it gets unhashed by l2tp_session_delete(), but
      it may test ->sk after the later got reset. The race is not so hard to
      trigger and syzbot found a pretty reliable reproducer:
      https://syzkaller.appspot.com/bug?id=418578d2a4389074524e04d641eacb091961b2cf
      
      Before d02ba2a6, another race could let pppol2tp_release()
      overwrite the ->__sk pointer of an L2TP session, thus tricking
      pppol2tp_put_sk() into calling sock_put() on a socket that is different
      than the one for which pppol2tp_release() was originally called. To get
      there, we had to trigger the race described above, therefore having one
      PPPoL2TP socket being released, while the session it is connected to is
      reconnecting to a different PPPoL2TP socket. When releasing this new
      socket fast enough, pppol2tp_release() overwrites the session's
      ->__sk pointer with the address of the new socket, before the first
      pppol2tp_put_sk() call gets scheduled. Then the pppol2tp_put_sk() call
      invoked by the original socket will sock_put() the new socket,
      potentially dropping its last reference. When the second
      pppol2tp_put_sk() finally runs, its socket has already been freed.
      
      With d02ba2a6, the session takes a reference on both sockets.
      Furthermore, the session's ->sk pointer is reset in the
      pppol2tp_session_close() callback function rather than in
      pppol2tp_release(). Therefore, ->__sk can't be overwritten and
      pppol2tp_put_sk() is called only once (l2tp_session_delete() will only
      run pppol2tp_session_close() once, to protect the session against
      concurrent deletion requests). Now pppol2tp_put_sk() will properly
      sock_put() the original socket, but the new socket will remain, as
      l2tp_session_delete() prevented the release process from completing.
      Here, we don't depend on the ->__sk race to trigger the bug. Getting
      into the pppol2tp_connect() race is enough to leak the reference, no
      matter when new socket is released.
      
      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 d02ba2a6) and checks that
      neither ->sk nor ->__sk is set before allowing a session to be
      connected.
      
      Fixes: d02ba2a6 ("l2tp: fix race in pppol2tp_release with session object destroy")
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      3d609342
  11. 26 5月, 2018 1 次提交
  12. 16 5月, 2018 1 次提交
  13. 27 4月, 2018 1 次提交
  14. 24 4月, 2018 1 次提交
  15. 23 4月, 2018 1 次提交
    • G
      l2tp: fix {pppol2tp, l2tp_dfs}_seq_stop() in case of seq_file overflow · 5411b618
      Guillaume Nault 提交于
      Commit 0e0c3fee ("l2tp: hold reference on tunnels printed in pppol2tp proc file")
      assumed that if pppol2tp_seq_stop() was called with non-NULL private
      data (the 'v' pointer), then pppol2tp_seq_start() would not be called
      again. It turns out that this isn't guaranteed, and overflowing the
      seq_file's buffer in pppol2tp_seq_show() is a way to get into this
      situation.
      
      Therefore, pppol2tp_seq_stop() needs to reset pd->tunnel, so that
      pppol2tp_seq_start() won't drop a reference again if it gets called.
      We also have to clear pd->session, because the rest of the code expects
      a non-NULL tunnel when pd->session is set.
      
      The l2tp_debugfs module has the same issue. Fix it in the same way.
      
      Fixes: 0e0c3fee ("l2tp: hold reference on tunnels printed in pppol2tp proc file")
      Fixes: f726214d ("l2tp: hold reference on tunnels printed in l2tp/tunnels debugfs file")
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      5411b618
  16. 14 4月, 2018 3 次提交
    • G
      l2tp: hold reference on tunnels printed in l2tp/tunnels debugfs file · f726214d
      Guillaume Nault 提交于
      Use l2tp_tunnel_get_nth() instead of l2tp_tunnel_find_nth(), to be safe
      against concurrent tunnel deletion.
      
      Use the same mechanism as in l2tp_ppp.c for dropping the reference
      taken by l2tp_tunnel_get_nth(). That is, drop the reference just
      before looking up the next tunnel. In case of error, drop the last
      accessed tunnel in l2tp_dfs_seq_stop().
      
      That was the last use of l2tp_tunnel_find_nth().
      
      Fixes: 0ad66140 ("l2tp: Add debugfs files for dumping l2tp debug info")
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      f726214d
    • G
      l2tp: hold reference on tunnels printed in pppol2tp proc file · 0e0c3fee
      Guillaume Nault 提交于
      Use l2tp_tunnel_get_nth() instead of l2tp_tunnel_find_nth(), to be safe
      against concurrent tunnel deletion.
      
      Unlike sessions, we can't drop the reference held on tunnels in
      pppol2tp_seq_show(). Tunnels are reused across several calls to
      pppol2tp_seq_start() when iterating over sessions. These iterations
      need the tunnel for accessing the next session. Therefore the only safe
      moment for dropping the reference is just before searching for the next
      tunnel.
      
      Normally, the last invocation of pppol2tp_next_tunnel() doesn't find
      any new tunnel, so it drops the last tunnel without taking any new
      reference. However, in case of error, pppol2tp_seq_stop() is called
      directly, so we have to drop the reference there.
      
      Fixes: fd558d18 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts")
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      0e0c3fee
    • G
      l2tp: hold reference on tunnels in netlink dumps · 5846c131
      Guillaume Nault 提交于
      l2tp_tunnel_find_nth() is unsafe: no reference is held on the returned
      tunnel, therefore it can be freed whenever the caller uses it.
      This patch defines l2tp_tunnel_get_nth() which works similarly, but
      also takes a reference on the returned tunnel. The caller then has to
      drop it after it stops using the tunnel.
      
      Convert netlink dumps to make them safe against concurrent tunnel
      deletion.
      
      Fixes: 309795f4 ("l2tp: Add netlink control API for L2TP")
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      5846c131
  17. 12 4月, 2018 2 次提交
    • G
      l2tp: fix race in duplicate tunnel detection · f6cd651b
      Guillaume Nault 提交于
      We can't use l2tp_tunnel_find() to prevent l2tp_nl_cmd_tunnel_create()
      from creating a duplicate tunnel. A tunnel can be concurrently
      registered after l2tp_tunnel_find() returns. Therefore, searching for
      duplicates must be done at registration time.
      
      Finally, remove l2tp_tunnel_find() entirely as it isn't use anywhere
      anymore.
      
      Fixes: 309795f4 ("l2tp: Add netlink control API for L2TP")
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      f6cd651b
    • G
      l2tp: fix races in tunnel creation · 6b9f3423
      Guillaume Nault 提交于
      l2tp_tunnel_create() inserts the new tunnel into the namespace's tunnel
      list and sets the socket's ->sk_user_data field, before returning it to
      the caller. Therefore, there are two ways the tunnel can be accessed
      and freed, before the caller even had the opportunity to take a
      reference. In practice, syzbot could crash the module by closing the
      socket right after a new tunnel was returned to pppol2tp_create().
      
      This patch moves tunnel registration out of l2tp_tunnel_create(), so
      that the caller can safely hold a reference before publishing the
      tunnel. This second step is done with the new l2tp_tunnel_register()
      function, which is now responsible for associating the tunnel to its
      socket and for inserting it into the namespace's list.
      
      While moving the code to l2tp_tunnel_register(), a few modifications
      have been done. First, the socket validation tests are done in a helper
      function, for clarity. Also, modifying the socket is now done after
      having inserted the tunnel to the namespace's tunnels list. This will
      allow insertion to fail, without having to revert theses modifications
      in the error path (a followup patch will check for duplicate tunnels
      before insertion). Either the socket is a kernel socket which we
      control, or it is a user-space socket for which we have a reference on
      the file descriptor. In any case, the socket isn't going to be closed
      from under us.
      
      Reported-by: syzbot+fbeeb5c3b538e8545644@syzkaller.appspotmail.com
      Fixes: fd558d18 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts")
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      6b9f3423
  18. 28 3月, 2018 1 次提交
  19. 27 3月, 2018 1 次提交
  20. 18 3月, 2018 1 次提交
  21. 13 3月, 2018 1 次提交
    • P
      l2tp: fix races with ipv4-mapped ipv6 addresses · b954f940
      Paolo Abeni 提交于
      The l2tp_tunnel_create() function checks for v4mapped ipv6
      sockets and cache that flag, so that l2tp core code can
      reusing it at xmit time.
      
      If the socket is provided by the userspace, the connection
      status of the tunnel sockets can change between the tunnel
      creation and the xmit call, so that syzbot is able to
      trigger the following splat:
      
      BUG: KASAN: use-after-free in ip6_dst_idev include/net/ip6_fib.h:192
      [inline]
      BUG: KASAN: use-after-free in ip6_xmit+0x1f76/0x2260
      net/ipv6/ip6_output.c:264
      Read of size 8 at addr ffff8801bd949318 by task syz-executor4/23448
      
      CPU: 0 PID: 23448 Comm: syz-executor4 Not tainted 4.16.0-rc4+ #65
      Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
      Google 01/01/2011
      Call Trace:
        __dump_stack lib/dump_stack.c:17 [inline]
        dump_stack+0x194/0x24d lib/dump_stack.c:53
        print_address_description+0x73/0x250 mm/kasan/report.c:256
        kasan_report_error mm/kasan/report.c:354 [inline]
        kasan_report+0x23c/0x360 mm/kasan/report.c:412
        __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
        ip6_dst_idev include/net/ip6_fib.h:192 [inline]
        ip6_xmit+0x1f76/0x2260 net/ipv6/ip6_output.c:264
        inet6_csk_xmit+0x2fc/0x580 net/ipv6/inet6_connection_sock.c:139
        l2tp_xmit_core net/l2tp/l2tp_core.c:1053 [inline]
        l2tp_xmit_skb+0x105f/0x1410 net/l2tp/l2tp_core.c:1148
        pppol2tp_sendmsg+0x470/0x670 net/l2tp/l2tp_ppp.c:341
        sock_sendmsg_nosec net/socket.c:630 [inline]
        sock_sendmsg+0xca/0x110 net/socket.c:640
        ___sys_sendmsg+0x767/0x8b0 net/socket.c:2046
        __sys_sendmsg+0xe5/0x210 net/socket.c:2080
        SYSC_sendmsg net/socket.c:2091 [inline]
        SyS_sendmsg+0x2d/0x50 net/socket.c:2087
        do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
        entry_SYSCALL_64_after_hwframe+0x42/0xb7
      RIP: 0033:0x453e69
      RSP: 002b:00007f819593cc68 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
      RAX: ffffffffffffffda RBX: 00007f819593d6d4 RCX: 0000000000453e69
      RDX: 0000000000000081 RSI: 000000002037ffc8 RDI: 0000000000000004
      RBP: 000000000072bea0 R08: 0000000000000000 R09: 0000000000000000
      R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
      R13: 00000000000004c3 R14: 00000000006f72e8 R15: 0000000000000000
      
      This change addresses the issues:
      * explicitly checking for TCP_ESTABLISHED for user space provided sockets
      * dropping the v4mapped flag usage - it can become outdated - and
        explicitly invoking ipv6_addr_v4mapped() instead
      
      The issue is apparently there since ancient times.
      
      v1 -> v2: (many thanks to Guillaume)
       - with csum issue introduced in v1
       - replace pr_err with pr_debug
       - fix build issue with IPV6 disabled
       - move l2tp_sk_is_v4mapped in l2tp_core.c
      
      v2 -> v3:
       - don't update inet_daddr for v4mapped address, unneeded
       - drop rendundant check at creation time
      
      Reported-and-tested-by: syzbot+92fa328176eb07e4ac1a@syzkaller.appspotmail.com
      Fixes: 3557baab ("[L2TP]: PPP over L2TP driver core")
      Signed-off-by: NPaolo Abeni <pabeni@redhat.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      b954f940
  22. 08 3月, 2018 1 次提交
    • E
      l2tp: do not accept arbitrary sockets · 17cfe79a
      Eric Dumazet 提交于
      syzkaller found an issue caused by lack of sufficient checks
      in l2tp_tunnel_create()
      
      RAW sockets can not be considered as UDP ones for instance.
      
      In another patch, we shall replace all pr_err() by less intrusive
      pr_debug() so that syzkaller can find other bugs faster.
      Acked-by: NGuillaume Nault <g.nault@alphalink.fr>
      Acked-by: NJames Chapman <jchapman@katalix.com>
      
      ==================================================================
      BUG: KASAN: slab-out-of-bounds in setup_udp_tunnel_sock+0x3ee/0x5f0 net/ipv4/udp_tunnel.c:69
      dst_release: dst:00000000d53d0d0f refcnt:-1
      Write of size 1 at addr ffff8801d013b798 by task syz-executor3/6242
      
      CPU: 1 PID: 6242 Comm: syz-executor3 Not tainted 4.16.0-rc2+ #253
      Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
      Call Trace:
       __dump_stack lib/dump_stack.c:17 [inline]
       dump_stack+0x194/0x24d lib/dump_stack.c:53
       print_address_description+0x73/0x250 mm/kasan/report.c:256
       kasan_report_error mm/kasan/report.c:354 [inline]
       kasan_report+0x23b/0x360 mm/kasan/report.c:412
       __asan_report_store1_noabort+0x17/0x20 mm/kasan/report.c:435
       setup_udp_tunnel_sock+0x3ee/0x5f0 net/ipv4/udp_tunnel.c:69
       l2tp_tunnel_create+0x1354/0x17f0 net/l2tp/l2tp_core.c:1596
       pppol2tp_connect+0x14b1/0x1dd0 net/l2tp/l2tp_ppp.c:707
       SYSC_connect+0x213/0x4a0 net/socket.c:1640
       SyS_connect+0x24/0x30 net/socket.c:1621
       do_syscall_64+0x280/0x940 arch/x86/entry/common.c:287
       entry_SYSCALL_64_after_hwframe+0x42/0xb7
      
      Fixes: fd558d18 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts")
      Signed-off-by: NEric Dumazet <edumazet@google.com>
      Reported-by: Nsyzbot <syzkaller@googlegroups.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      17cfe79a
  23. 28 2月, 2018 1 次提交