1. 06 Feb, 2019 2 commits
    • Jacob Wen's avatar
      l2tp: fix reading optional fields of L2TPv3 · cac9e3fb
      Jacob Wen authored
      [ Upstream commit 4522a70d ]
      
      Use pskb_may_pull() to make sure the optional fields are in skb linear
      parts, so we can safely read them later.
      
      It's easy to reproduce the issue with a net driver that supports paged
      skb data. Just create a L2TPv3 over IP tunnel and then generates some
      network traffic.
      Once reproduced, rx err in /sys/kernel/debug/l2tp/tunnels will increase.
      
      Changes in v4:
      1. s/l2tp_v3_pull_opt/l2tp_v3_ensure_opt_in_linear/
      2. s/tunnel->version != L2TP_HDR_VER_2/tunnel->version == L2TP_HDR_VER_3/
      3. Add 'Fixes' in commit messages.
      
      Changes in v3:
      1. To keep consistency, move the code out of l2tp_recv_common.
      2. Use "net" instead of "net-next", since this is a bug fix.
      
      Changes in v2:
      1. Only fix L2TPv3 to make code simple.
         To fix both L2TPv3 and L2TPv2, we'd better refactor l2tp_recv_common.
         It's complicated to do so.
      2. Reloading pointers after pskb_may_pull
      
      Fixes: f7faffa3 ("l2tp: Add L2TPv3 protocol support")
      Fixes: 0d76751f ("l2tp: Add L2TPv3 IP encapsulation (no UDP) support")
      Fixes: a32e0eec ("l2tp: introduce L2TPv3 IP encapsulation support for IPv6")
      Signed-off-by: default avatarJacob Wen <jian.w.wen@oracle.com>
      Acked-by: default avatarGuillaume Nault <gnault@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      cac9e3fb
    • Jacob Wen's avatar
      l2tp: copy 4 more bytes to linear part if necessary · 16ac09ec
      Jacob Wen authored
      [ Upstream commit 91c52470 ]
      
      The size of L2TPv2 header with all optional fields is 14 bytes.
      l2tp_udp_recv_core only moves 10 bytes to the linear part of a
      skb. This may lead to l2tp_recv_common read data outside of a skb.
      
      This patch make sure that there is at least 14 bytes in the linear
      part of a skb to meet the maximum need of l2tp_udp_recv_core and
      l2tp_recv_common. The minimum size of both PPP HDLC-like frame and
      Ethernet frame is larger than 14 bytes, so we are safe to do so.
      
      Also remove L2TP_HDR_SIZE_NOSEQ, it is unused now.
      
      Fixes: fd558d18 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts")
      Suggested-by: default avatarGuillaume Nault <gnault@redhat.com>
      Signed-off-by: default avatarJacob Wen <jian.w.wen@oracle.com>
      Acked-by: default avatarGuillaume Nault <gnault@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      16ac09ec
  2. 15 Nov, 2018 1 commit
  3. 14 Aug, 2018 1 commit
    • Arnd Bergmann's avatar
      l2tp: fix unused function warning · c2ebc256
      Arnd Bergmann authored
      Removing one of the callers of pppol2tp_session_get_sock caused a harmless
      warning in some configurations:
      
      net/l2tp/l2tp_ppp.c:142:21: 'pppol2tp_session_get_sock' defined but not used [-Wunused-function]
      
      Rather than adding another #ifdef here, using a proper IS_ENABLED()
      check makes the code more readable and avoids those warnings while
      letting the compiler figure out for itself which code is needed.
      
      This adds one pointer for the unused show() callback in struct
      l2tp_session, but that seems harmless.
      
      Fixes: b0e29063 ("l2tp: remove pppol2tp_session_ioctl()")
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      c2ebc256
  4. 13 Aug, 2018 1 commit
    • Wei Wang's avatar
      l2tp: use sk_dst_check() to avoid race on sk->sk_dst_cache · 6d37fa49
      Wei Wang authored
      In l2tp code, if it is a L2TP_UDP_ENCAP tunnel, tunnel->sk points to a
      UDP socket. User could call sendmsg() on both this tunnel and the UDP
      socket itself concurrently. As l2tp_xmit_skb() holds socket lock and call
      __sk_dst_check() to refresh sk->sk_dst_cache, while udpv6_sendmsg() is
      lockless and call sk_dst_check() to refresh sk->sk_dst_cache, there
      could be a race and cause the dst cache to be freed multiple times.
      So we fix l2tp side code to always call sk_dst_check() to garantee
      xchg() is called when refreshing sk->sk_dst_cache to avoid race
      conditions.
      
      Syzkaller reported stack trace:
      BUG: KASAN: use-after-free in atomic_read include/asm-generic/atomic-instrumented.h:21 [inline]
      BUG: KASAN: use-after-free in atomic_fetch_add_unless include/linux/atomic.h:575 [inline]
      BUG: KASAN: use-after-free in atomic_add_unless include/linux/atomic.h:597 [inline]
      BUG: KASAN: use-after-free in dst_hold_safe include/net/dst.h:308 [inline]
      BUG: KASAN: use-after-free in ip6_hold_safe+0xe6/0x670 net/ipv6/route.c:1029
      Read of size 4 at addr ffff8801aea9a880 by task syz-executor129/4829
      
      CPU: 0 PID: 4829 Comm: syz-executor129 Not tainted 4.18.0-rc7-next-20180802+ #30
      Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
      Call Trace:
       __dump_stack lib/dump_stack.c:77 [inline]
       dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
       print_address_description+0x6c/0x20b mm/kasan/report.c:256
       kasan_report_error mm/kasan/report.c:354 [inline]
       kasan_report.cold.7+0x242/0x30d mm/kasan/report.c:412
       check_memory_region_inline mm/kasan/kasan.c:260 [inline]
       check_memory_region+0x13e/0x1b0 mm/kasan/kasan.c:267
       kasan_check_read+0x11/0x20 mm/kasan/kasan.c:272
       atomic_read include/asm-generic/atomic-instrumented.h:21 [inline]
       atomic_fetch_add_unless include/linux/atomic.h:575 [inline]
       atomic_add_unless include/linux/atomic.h:597 [inline]
       dst_hold_safe include/net/dst.h:308 [inline]
       ip6_hold_safe+0xe6/0x670 net/ipv6/route.c:1029
       rt6_get_pcpu_route net/ipv6/route.c:1249 [inline]
       ip6_pol_route+0x354/0xd20 net/ipv6/route.c:1922
       ip6_pol_route_output+0x54/0x70 net/ipv6/route.c:2098
       fib6_rule_lookup+0x283/0x890 net/ipv6/fib6_rules.c:122
       ip6_route_output_flags+0x2c5/0x350 net/ipv6/route.c:2126
       ip6_dst_lookup_tail+0x1278/0x1da0 net/ipv6/ip6_output.c:978
       ip6_dst_lookup_flow+0xc8/0x270 net/ipv6/ip6_output.c:1079
       ip6_sk_dst_lookup_flow+0x5ed/0xc50 net/ipv6/ip6_output.c:1117
       udpv6_sendmsg+0x2163/0x36b0 net/ipv6/udp.c:1354
       inet_sendmsg+0x1a1/0x690 net/ipv4/af_inet.c:798
       sock_sendmsg_nosec net/socket.c:622 [inline]
       sock_sendmsg+0xd5/0x120 net/socket.c:632
       ___sys_sendmsg+0x51d/0x930 net/socket.c:2115
       __sys_sendmmsg+0x240/0x6f0 net/socket.c:2210
       __do_sys_sendmmsg net/socket.c:2239 [inline]
       __se_sys_sendmmsg net/socket.c:2236 [inline]
       __x64_sys_sendmmsg+0x9d/0x100 net/socket.c:2236
       do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
       entry_SYSCALL_64_after_hwframe+0x49/0xbe
      RIP: 0033:0x446a29
      Code: e8 ac b8 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 eb 08 fc ff c3 66 2e 0f 1f 84 00 00 00 00
      RSP: 002b:00007f4de5532db8 EFLAGS: 00000246 ORIG_RAX: 0000000000000133
      RAX: ffffffffffffffda RBX: 00000000006dcc38 RCX: 0000000000446a29
      RDX: 00000000000000b8 RSI: 0000000020001b00 RDI: 0000000000000003
      RBP: 00000000006dcc30 R08: 00007f4de5533700 R09: 0000000000000000
      R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006dcc3c
      R13: 00007ffe2b830fdf R14: 00007f4de55339c0 R15: 0000000000000001
      
      Fixes: 71b1391a ("l2tp: ensure sk->dst is still valid")
      Reported-by: syzbot+05f840f3b04f211bad55@syzkaller.appspotmail.com
      Signed-off-by: default avatarWei Wang <weiwan@google.com>
      Signed-off-by: default avatarMartin KaFai Lau <kafai@fb.com>
      Cc: Guillaume Nault <g.nault@alphalink.fr>
      Cc: David Ahern <dsahern@gmail.com>
      Cc: Cong Wang <xiyou.wangcong@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      6d37fa49
  5. 11 Aug, 2018 8 commits
  6. 03 Aug, 2018 4 commits
    • Guillaume Nault's avatar
      l2tp: fix missing refcount drop in pppol2tp_tunnel_ioctl() · f664e37d
      Guillaume Nault authored
      If 'session' is not NULL and is not a PPP pseudo-wire, then we fail to
      drop the reference taken by l2tp_session_get().
      
      Fixes: ecd012e4 ("l2tp: filter out non-PPP sessions in pppol2tp_tunnel_ioctl()")
      Signed-off-by: default avatarGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f664e37d
    • Guillaume Nault's avatar
      l2tp: ignore L2TP_ATTR_MTU · e9697e2e
      Guillaume Nault authored
      This attribute's handling is broken. It can only be used when creating
      Ethernet pseudo-wires, in which case its value can be used as the
      initial MTU for the l2tpeth device.
      However, when handling update requests, L2TP_ATTR_MTU only modifies
      session->mtu. This value is never propagated to the l2tpeth device.
      Dump requests also return the value of session->mtu, which is not
      synchronised anymore with the device MTU.
      
      The same problem occurs if the device MTU is properly updated using the
      generic IFLA_MTU attribute. In this case, session->mtu is not updated,
      and L2TP_ATTR_MTU will report an invalid value again when dumping the
      session.
      
      It does not seem worthwhile to complexify l2tp_eth.c to synchronise
      session->mtu with the device MTU. Even the ip-l2tp manpage advises to
      use 'ip link' to initialise the MTU of l2tpeth devices (iproute2 does
      not handle L2TP_ATTR_MTU at all anyway). So let's just ignore it
      entirely.
      Signed-off-by: default avatarGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e9697e2e
    • Guillaume Nault's avatar
      l2tp: simplify MTU handling in l2tp_ppp · 789141b2
      Guillaume Nault authored
      The value of the session's .mtu field, as defined by
      pppol2tp_connect() or pppol2tp_session_create(), is later overwritten
      by pppol2tp_session_init() (unless getting the tunnel's socket PMTU
      fails). This field is then only used when setting the PPP channel's MTU
      in pppol2tp_connect().
      Furthermore, the SIOC[GS]IFMTU ioctls only act on the session's .mtu
      without propagating this value to the PPP channel, making them useless.
      
      This patch initialises the PPP channel's MTU directly and ignores the
      session's .mtu entirely. MTU is still computed by subtracting the
      PPPOL2TP_HEADER_OVERHEAD constant. It is not optimal, but that doesn't
      really matter: po->chan.mtu is only used when the channel is part of a
      multilink PPP bundle. Running multilink PPP over packet switched
      networks is certainly not going to be efficient, so not picking the
      best MTU does not harm (in the worst case, packets will just be
      fragmented by the underlay).
      
      The SIOC[GS]IFMTU ioctls are removed entirely (as opposed to simply
      ignored), because these ioctls commands are part of the requests that
      should be handled generically by the socket layer. PX_PROTO_OL2TP was
      the only socket type abusing these ioctls.
      Signed-off-by: default avatarGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      789141b2
    • Guillaume Nault's avatar
      l2tp: define l2tp_tunnel_dst_mtu() · 1f5cd2a0
      Guillaume Nault authored
      Consolidate retrieval of tunnel's socket mtu in order to simplify
      l2tp_eth and l2tp_ppp a bit.
      Signed-off-by: default avatarGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      1f5cd2a0
  7. 27 Jul, 2018 4 commits
  8. 26 Jul, 2018 1 commit
    • Guillaume Nault's avatar
      l2tp: remove ->recv_payload_hook · 2b139e6b
      Guillaume Nault authored
      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: default avatarGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      2b139e6b
  9. 24 Jul, 2018 1 commit
  10. 07 Jul, 2018 2 commits
  11. 28 Jun, 2018 2 commits
    • Linus Torvalds's avatar
      Revert changes to convert to ->poll_mask() and aio IOCB_CMD_POLL · a11e1d43
      Linus Torvalds authored
      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: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      a11e1d43
    • Guillaume Nault's avatar
      l2tp: define helper for parsing struct sockaddr_pppol2tp* · a408194a
      Guillaume Nault authored
      '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: default avatarGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      a408194a
  12. 26 Jun, 2018 7 commits
  13. 15 Jun, 2018 6 commits