14 Apr, 2018

2 commits

  • 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: 0ad6614048cf ("l2tp: Add debugfs files for dumping l2tp debug info")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    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: 309795f4bec2 ("l2tp: Add netlink control API for L2TP")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

12 Apr, 2018

2 commits

  • 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: 309795f4bec2 ("l2tp: Add netlink control API for L2TP")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    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: fd558d186df2 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

13 Mar, 2018

1 commit

  • 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: 3557baabf280 ("[L2TP]: PPP over L2TP driver core")
    Signed-off-by: Paolo Abeni
    Signed-off-by: David S. Miller

    Paolo Abeni
     

27 Feb, 2018

1 commit

  • The tunnel socket tunnel->sock (struct sock) is accessed when
    preparing a new ppp session on a tunnel at pppol2tp_session_init. If
    the socket is closed by a thread while another is creating a new
    session, the threads race. In pppol2tp_connect, the tunnel object may
    be created if the pppol2tp socket is associated with the special
    session_id 0 and the tunnel socket is looked up using the provided
    fd. When handling this, pppol2tp_connect cannot sock_hold the tunnel
    socket to prevent it being destroyed during pppol2tp_connect since
    this may itself may race with the socket being destroyed. Doing
    sockfd_lookup in pppol2tp_connect isn't sufficient to prevent
    tunnel->sock going away either because a given tunnel socket fd may be
    reused between calls to pppol2tp_connect. Instead, have
    l2tp_tunnel_create sock_hold the tunnel socket before it does
    sockfd_put. This ensures that the tunnel's socket is always extant
    while the tunnel object exists. Hold a ref on the socket until the
    tunnel is destroyed and ensure that all tunnel destroy paths go
    through a common function (l2tp_tunnel_delete) since this will do the
    final sock_put to release the tunnel socket.

    Since the tunnel's socket is now guaranteed to exist if the tunnel
    exists, we no longer need to use sockfd_lookup via l2tp_sock_to_tunnel
    to derive the tunnel from the socket since this is always
    sk_user_data.

    Also, sessions no longer sock_hold the tunnel socket since sessions
    already hold a tunnel ref and the tunnel sock will not be freed until
    the tunnel is freed. Removing these sock_holds in
    l2tp_session_register avoids a possible sock leak in the
    pppol2tp_connect error path if l2tp_session_register succeeds but
    attaching a ppp channel fails. The pppol2tp_connect error path could
    have been fixed instead and have the sock ref dropped when the session
    is freed, but doing a sock_put of the tunnel socket when the session
    is freed would require a new session_free callback. It is simpler to
    just remove the sock_hold of the tunnel socket in
    l2tp_session_register, now that the tunnel socket lifetime is
    guaranteed.

    Finally, some init code in l2tp_tunnel_create is reordered to ensure
    that the new tunnel object's refcount is set and the tunnel socket ref
    is taken before the tunnel socket destructor callbacks are set.

    kasan: CONFIG_KASAN_INLINE enabled
    kasan: GPF could be caused by NULL-ptr deref or user memory access
    general protection fault: 0000 [#1] SMP KASAN
    Modules linked in:
    CPU: 0 PID: 4360 Comm: syzbot_19c09769 Not tainted 4.16.0-rc2+ #34
    Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
    RIP: 0010:pppol2tp_session_init+0x1d6/0x500
    RSP: 0018:ffff88001377fb40 EFLAGS: 00010212
    RAX: dffffc0000000000 RBX: ffff88001636a940 RCX: ffffffff84836c1d
    RDX: 0000000000000045 RSI: 0000000055976744 RDI: 0000000000000228
    RBP: ffff88001377fb60 R08: ffffffff84836bc8 R09: 0000000000000002
    R10: ffff88001377fab8 R11: 0000000000000001 R12: 0000000000000000
    R13: ffff88001636aac8 R14: ffff8800160f81c0 R15: 1ffff100026eff76
    FS: 00007ffb3ea66700(0000) GS:ffff88001a400000(0000) knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 0000000020e77000 CR3: 0000000016261000 CR4: 00000000000006f0
    Call Trace:
    pppol2tp_connect+0xd18/0x13c0
    ? pppol2tp_session_create+0x170/0x170
    ? __might_fault+0x115/0x1d0
    ? lock_downgrade+0x860/0x860
    ? __might_fault+0xe5/0x1d0
    ? security_socket_connect+0x8e/0xc0
    SYSC_connect+0x1b6/0x310
    ? SYSC_bind+0x280/0x280
    ? __do_page_fault+0x5d1/0xca0
    ? up_read+0x1f/0x40
    ? __do_page_fault+0x3c8/0xca0
    SyS_connect+0x29/0x30
    ? SyS_accept+0x40/0x40
    do_syscall_64+0x1e0/0x730
    ? trace_hardirqs_off_thunk+0x1a/0x1c
    entry_SYSCALL_64_after_hwframe+0x42/0xb7
    RIP: 0033:0x7ffb3e376259
    RSP: 002b:00007ffeda4f6508 EFLAGS: 00000202 ORIG_RAX: 000000000000002a
    RAX: ffffffffffffffda RBX: 0000000020e77012 RCX: 00007ffb3e376259
    RDX: 000000000000002e RSI: 0000000020e77000 RDI: 0000000000000004
    RBP: 00007ffeda4f6540 R08: 0000000000000000 R09: 0000000000000000
    R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000400b60
    R13: 00007ffeda4f6660 R14: 0000000000000000 R15: 0000000000000000
    Code: 80 3d b0 ff 06 02 00 0f 84 07 02 00 00 e8 13 d6 db fc 49 8d bc 24 28 02 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 f
    a 48 c1 ea 03 3c 02 00 0f 85 ed 02 00 00 4d 8b a4 24 28 02 00 00 e8 13 16

    Fixes: 80d84ef3ff1dd ("l2tp: prevent l2tp_tunnel_delete racing with userspace close")
    Signed-off-by: James Chapman
    Signed-off-by: David S. Miller

    James Chapman
     

20 Jan, 2018

2 commits

  • Remove l2specific_len configuration parameter since now L2-Specific
    Sublayer length is computed according to l2specific_type provided by
    userspace.

    Reviewed-by: Guillaume Nault
    Tested-by: Guillaume Nault
    Signed-off-by: Lorenzo Bianconi
    Signed-off-by: David S. Miller

    Lorenzo Bianconi
     
  • Remove l2specific_len dependency while building l2tpv3 header or
    parsing the received frame since default L2-Specific Sublayer is
    always four bytes long and we don't need to rely on a user supplied
    value.
    Moreover in l2tp netlink code there are no sanity checks to
    enforce the relation between l2specific_len and l2specific_type,
    so sending a malformed netlink message is possible to set
    l2specific_type to L2TP_L2SPECTYPE_DEFAULT (or even
    L2TP_L2SPECTYPE_NONE) and set l2specific_len to a value greater than
    4 leaking memory on the wire and sending corrupted frames.

    Reviewed-by: Guillaume Nault
    Tested-by: Guillaume Nault
    Signed-off-by: Lorenzo Bianconi
    Signed-off-by: David S. Miller

    Lorenzo Bianconi
     

06 Jan, 2018

2 commits

  • If L2TP_ATTR_OFFSET is set to a non-zero value in L2TPv3 tunnels, it
    results in L2TPv3 packets being transmitted which might not be
    compliant with the L2TPv3 RFC. This patch has l2tp ignore the offset
    setting and send all packets with no offset.

    In more detail:

    L2TPv2 supports a variable offset from the L2TPv2 header to the
    payload. The offset value is indicated by an optional field in the
    L2TP header. Our L2TP implementation already detects the presence of
    the optional offset and skips that many bytes when handling data
    received packets. All transmitted packets are always transmitted with
    no offset.

    L2TPv3 has no optional offset field in the L2TPv3 packet
    header. Instead, L2TPv3 defines optional fields in a "Layer-2 Specific
    Sublayer". At the time when the original L2TP code was written, there
    was talk at IETF of offset being implemented in a new Layer-2 Specific
    Sublayer. A L2TP_ATTR_OFFSET netlink attribute was added so that this
    offset could be configured and the intention was to allow it to be
    also used to set the tx offset for L2TPv2. However, no L2TPv3 offset
    was ever specified and the L2TP_ATTR_OFFSET parameter was forgotten
    about.

    Setting L2TP_ATTR_OFFSET results in L2TPv3 packets being transmitted
    with the specified number of bytes padding between L2TPv3 header and
    payload. This is not compliant with L2TPv3 RFC3931. This change
    removes the configurable offset altogether while retaining
    L2TP_ATTR_OFFSET for backwards compatibility. Any L2TP_ATTR_OFFSET
    value is ignored.

    Signed-off-by: James Chapman
    Signed-off-by: David S. Miller

    James Chapman
     
  • Revert commit f15bc54eeecd ("l2tp: add peer_offset parameter"). This
    is removed because it is adding another configurable offset and
    configurable offsets are being removed.

    Signed-off-by: James Chapman
    Signed-off-by: David S. Miller

    James Chapman
     

28 Dec, 2017

1 commit

  • Introduce peer_offset parameter in order to add the capability
    to specify two different values for payload offset on tx/rx side.
    If just offset is provided by userspace use it for rx side as well
    in order to maintain compatibility with older l2tp versions

    Signed-off-by: Lorenzo Bianconi
    Signed-off-by: David S. Miller

    Lorenzo Bianconi
     

01 Nov, 2017

2 commits

  • With conversion to refcount_t, such manual debugging code doesn't make
    sense anymore.
    The tunnel part was already dropped by
    54652eb12c1b ("l2tp: hold tunnel while looking up sessions in l2tp_netlink").

    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     
  • The ->ref() and ->deref() callbacks are unused since PPP stopped using
    them in ee40fb2e1eb5 ("l2tp: protect sock pointer of struct pppol2tp_session with RCU").

    We can thus remove them from struct l2tp_session and drop the do_ref
    parameter of l2tp_session_get*().

    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

29 Oct, 2017

1 commit

  • Sessions created by l2tp_session_create() aren't fully initialised:
    some pseudo-wire specific operations need to be done before making the
    session usable. Therefore the PPP and Ethernet pseudo-wires continue
    working on the returned l2tp session while it's already been exposed to
    the rest of the system.
    This can lead to various issues. In particular, the session may enter
    the deletion process before having been fully initialised, which will
    confuse the session removal code.

    This patch moves session registration out of l2tp_session_create(), so
    that callers can control when the session is exposed to the rest of the
    system. This is done by the new l2tp_session_register() function.

    Only pppol2tp_session_create() can be easily converted to avoid
    modifying its session after registration (the debug message is dropped
    in order to avoid the need for holding a reference on the session).

    For pppol2tp_connect() and l2tp_eth_create()), more work is needed.
    That'll be done in followup patches. For now, let's just register the
    session right after its creation, like it was done before. The only
    difference is that we can easily take a reference on the session before
    registering it, so, at least, we're sure it's not going to be freed
    while we're working on it.

    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

27 Sep, 2017

1 commit

  • If we try to delete the same tunnel twice, the first delete operation
    does a lookup (l2tp_tunnel_get), finds the tunnel, calls
    l2tp_tunnel_delete, which queues it for deletion by
    l2tp_tunnel_del_work.

    The second delete operation also finds the tunnel and calls
    l2tp_tunnel_delete. If the workqueue has already fired and started
    running l2tp_tunnel_del_work, then l2tp_tunnel_delete will queue the
    same tunnel a second time, and try to free the socket again.

    Add a dead flag to prevent firing the workqueue twice. Then we can
    remove the check of queue_work's result that was meant to prevent that
    race but doesn't.

    Reproducer:

    ip l2tp add tunnel tunnel_id 3000 peer_tunnel_id 4000 local 192.168.0.2 remote 192.168.0.1 encap udp udp_sport 5000 udp_dport 6000
    ip l2tp add session name l2tp1 tunnel_id 3000 session_id 1000 peer_session_id 2000
    ip link set l2tp1 up
    ip l2tp del tunnel tunnel_id 3000
    ip l2tp del tunnel tunnel_id 3000

    Fixes: f8ccac0e4493 ("l2tp: put tunnel socket release on a workqueue")
    Reported-by: Jianlin Shi
    Signed-off-by: Sabrina Dubroca
    Acked-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Sabrina Dubroca
     

26 Sep, 2017

1 commit

  • There are several ways to remove L2TP sessions:

    * deleting a session explicitly using the netlink interface (with
    L2TP_CMD_SESSION_DELETE),
    * deleting the session's parent tunnel (either by closing the
    tunnel's file descriptor or using the netlink interface),
    * closing the PPPOL2TP file descriptor of a PPP pseudo-wire.

    In some cases, when these methods are used concurrently on the same
    session, the session can be removed twice, leading to use-after-free
    bugs.

    This patch adds a 'dead' flag, used by l2tp_session_delete() and
    l2tp_tunnel_closeall() to prevent them from stepping on each other's
    toes.

    The session deletion path used when closing a PPPOL2TP file descriptor
    doesn't need to be adapted. It already has to ensure that a session
    remains valid for the lifetime of its PPPOL2TP file descriptor.
    So it takes an extra reference on the session in the ->session_close()
    callback (pppol2tp_session_close()), which is eventually dropped
    in the ->sk_destruct() callback of the PPPOL2TP socket
    (pppol2tp_session_destruct()).
    Still, __l2tp_session_unhash() and l2tp_session_queue_purge() can be
    called twice and even concurrently for a given session, but thanks to
    proper locking and re-initialisation of list fields, this is not an
    issue.

    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

04 Sep, 2017

2 commits

  • Using l2tp_tunnel_find() in pppol2tp_session_create() and
    l2tp_eth_create() is racy, because no reference is held on the
    returned session. These functions are only used to implement the
    ->session_create callback which is run by l2tp_nl_cmd_session_create().
    Therefore searching for the parent tunnel isn't necessary because
    l2tp_nl_cmd_session_create() already has a pointer to it and holds a
    reference.

    This patch modifies ->session_create()'s prototype to directly pass the
    the parent tunnel as parameter, thus avoiding searching for it in
    pppol2tp_session_create() and l2tp_eth_create().

    Since we have to touch the ->session_create() call in
    l2tp_nl_cmd_session_create(), let's also remove the useless conditional:
    we know that ->session_create isn't NULL at this point because it's
    already been checked earlier in this same function.

    Finally, one might be tempted to think that the removed
    l2tp_tunnel_find() calls were harmless because they would return the
    same tunnel as the one held by l2tp_nl_cmd_session_create() anyway.
    But that tunnel might be removed and a new one created with same tunnel
    Id before the l2tp_tunnel_find() call. In this case l2tp_tunnel_find()
    would return the new tunnel which wouldn't be protected by the
    reference held by l2tp_nl_cmd_session_create().

    Fixes: 309795f4bec2 ("l2tp: Add netlink control API for L2TP")
    Fixes: d9e31d17ceba ("l2tp: Add L2TP ethernet pseudowire support")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     
  • l2tp_tunnel_destruct() sets tunnel->sock to NULL, then removes the
    tunnel from the pernet list and finally closes all its sessions.
    Therefore, it's possible to add a session to a tunnel that is still
    reachable, but for which tunnel->sock has already been reset. This can
    make l2tp_session_create() dereference a NULL pointer when calling
    sock_hold(tunnel->sock).

    This patch adds the .acpt_newsess field to struct l2tp_tunnel, which is
    used by l2tp_tunnel_closeall() to prevent addition of new sessions to
    tunnels. Resetting tunnel->sock is done after l2tp_tunnel_closeall()
    returned, so that l2tp_session_add_to_tunnel() can safely take a
    reference on it when .acpt_newsess is true.

    The .acpt_newsess field is modified in l2tp_tunnel_closeall(), rather
    than in l2tp_tunnel_destruct(), so that it benefits all tunnel removal
    mechanisms. E.g. on UDP tunnels, a session could be added to a tunnel
    after l2tp_udp_encap_destroy() proceeded. This would prevent the tunnel
    from being removed because of the references held by this new session
    on the tunnel and its socket. Even though the session could be removed
    manually later on, this defeats the purpose of
    commit 9980d001cec8 ("l2tp: add udp encap socket destroy handler").

    Fixes: fd558d186df2 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

29 Aug, 2017

1 commit

  • l2tp_tunnel_find() doesn't take a reference on the returned tunnel.
    Therefore, it's unsafe to use it because the returned tunnel can go
    away on us anytime.

    Fix this by defining l2tp_tunnel_get(), which works like
    l2tp_tunnel_find(), but takes a reference on the returned tunnel.
    Caller then has to drop this reference using l2tp_tunnel_dec_refcount().

    As l2tp_tunnel_dec_refcount() needs to be moved to l2tp_core.h, let's
    simplify the patch and not move the L2TP_REFCNT_DEBUG part. This code
    has been broken (not even compiling) in May 2012 by
    commit a4ca44fa578c ("net: l2tp: Standardize logging styles")
    and fixed more than two years later by
    commit 29abe2fda54f ("l2tp: fix missing line continuation"). So it
    doesn't appear to be used by anyone.

    Same thing for l2tp_tunnel_free(); instead of moving it to l2tp_core.h,
    let's just simplify things and call kfree_rcu() directly in
    l2tp_tunnel_dec_refcount(). Extra assertions and debugging code
    provided by l2tp_tunnel_free() didn't help catching any of the
    reference counting and socket handling issues found while working on
    this series.

    Fixes: 309795f4bec2 ("l2tp: Add netlink control API for L2TP")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

05 Jul, 2017

2 commits


12 Apr, 2017

3 commits


05 Apr, 2017

1 commit

  • Take a reference on the sessions returned by l2tp_session_find_nth()
    (and rename it l2tp_session_get_nth() to reflect this change), so that
    caller is assured that the session isn't going to disappear while
    processing it.

    For procfs and debugfs handlers, the session is held in the .start()
    callback and dropped in .show(). Given that pppol2tp_seq_session_show()
    dereferences the associated PPPoL2TP socket and that
    l2tp_dfs_seq_session_show() might call pppol2tp_show(), we also need to
    call the session's .ref() callback to prevent the socket from going
    away from under us.

    Fixes: fd558d186df2 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts")
    Fixes: 0ad6614048cf ("l2tp: Add debugfs files for dumping l2tp debug info")
    Fixes: 309795f4bec2 ("l2tp: Add netlink control API for L2TP")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

02 Apr, 2017

2 commits

  • Callers of l2tp_nl_session_find() need to hold a reference on the
    returned session since there's no guarantee that it isn't going to
    disappear from under them.

    Relying on the fact that no l2tp netlink message may be processed
    concurrently isn't enough: sessions can be deleted by other means
    (e.g. by closing the PPPOL2TP socket of a ppp pseudowire).

    l2tp_nl_cmd_session_delete() is a bit special: it runs a callback
    function that may require a previous call to session->ref(). In
    particular, for ppp pseudowires, the callback is l2tp_session_delete(),
    which then calls pppol2tp_session_close() and dereferences the PPPOL2TP
    socket. The socket might already be gone at the moment
    l2tp_session_delete() calls session->ref(), so we need to take a
    reference during the session lookup. So we need to pass the do_ref
    variable down to l2tp_session_get() and l2tp_session_get_by_ifname().

    Since all callers have to be updated, l2tp_session_find_by_ifname() and
    l2tp_nl_session_find() are renamed to reflect their new behaviour.

    Fixes: 309795f4bec2 ("l2tp: Add netlink control API for L2TP")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     
  • Taking a reference on sessions in l2tp_recv_common() is racy; this
    has to be done by the callers.

    To this end, a new function is required (l2tp_session_get()) to
    atomically lookup a session and take a reference on it. Callers then
    have to manually drop this reference.

    Fixes: fd558d186df2 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

11 Feb, 2017

1 commit

  • udp_ioctl(), as its name suggests, is used by UDP protocols,
    but is also used by L2TP :(

    L2TP should use its own handler, because it really does not
    look the same.

    SIOCINQ for instance should not assume UDP checksum or headers.

    Thanks to Andrey and syzkaller team for providing the report
    and a nice reproducer.

    While crashes only happen on recent kernels (after commit
    7c13f97ffde6 ("udp: do fwd memory scheduling on dequeue")), this
    probably needs to be backported to older kernels.

    Fixes: 7c13f97ffde6 ("udp: do fwd memory scheduling on dequeue")
    Fixes: 85584672012e ("udp: Fix udp_poll() and ioctl()")
    Signed-off-by: Eric Dumazet
    Reported-by: Andrey Konovalov
    Acked-by: Paolo Abeni
    Signed-off-by: David S. Miller

    Eric Dumazet
     

11 Dec, 2016

1 commit


11 Sep, 2016

1 commit


26 Sep, 2015

1 commit

  • It should not be necessary to do explicit module loading when
    configuring L2TP. Modules should be loaded as needed instead
    (as is done already with netlink and other tunnel types).

    This patch adds a new module alias type and code to load
    the sub module on demand.

    Signed-off-by: Stephen Hemminger
    Signed-off-by: David S. Miller

    stephen hemminger
     

24 May, 2014

1 commit


07 Mar, 2014

1 commit

  • Commit e0d4435f "l2tp: Update PPP-over-L2TP driver to work over L2TPv3"
    broke the PPPOL2TP_SO_SENDSEQ setsockopt. The L2TP header length was
    previously computed by pppol2tp_l2t_header_len() before each call to
    l2tp_xmit_skb(). Now that header length is retrieved from the hdr_len
    session field, this field must be updated every time the L2TP header
    format is modified, or l2tp_xmit_skb() won't push the right amount of
    data for the L2TP header.

    This patch uses l2tp_session_set_header_len() to adjust hdr_len every
    time sequencing is (de)activated from userspace (either by the
    PPPOL2TP_SO_SENDSEQ setsockopt or the L2TP_ATTR_SEND_SEQ netlink
    attribute).

    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

14 Jan, 2014

1 commit


20 Oct, 2013

1 commit

  • There are a mix of function prototypes with and without extern
    in the kernel sources. Standardize on not using extern for
    function prototypes.

    Function prototypes don't need to be written with extern.
    extern is assumed by the compiler. Its use is as unnecessary as
    using auto to declare automatic/local variables in a block.

    Signed-off-by: Joe Perches
    Signed-off-by: David S. Miller

    Joe Perches
     

03 Oct, 2013

1 commit

  • IPv4 mapped addresses cause kernel panic.
    The patch juste check whether the IPv6 address is an IPv4 mapped
    address. If so, use IPv4 API instead of IPv6.

    [ 940.026915] general protection fault: 0000 [#1]
    [ 940.026915] Modules linked in: l2tp_ppp l2tp_netlink l2tp_core pppox ppp_generic slhc loop psmouse
    [ 940.026915] CPU: 0 PID: 3184 Comm: memcheck-amd64- Not tainted 3.11.0+ #1
    [ 940.026915] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
    [ 940.026915] task: ffff880007130e20 ti: ffff88000737e000 task.ti: ffff88000737e000
    [ 940.026915] RIP: 0010:[] [] ip6_xmit+0x276/0x326
    [ 940.026915] RSP: 0018:ffff88000737fd28 EFLAGS: 00010286
    [ 940.026915] RAX: c748521a75ceff48 RBX: ffff880000c30800 RCX: 0000000000000000
    [ 940.026915] RDX: ffff88000075cc4e RSI: 0000000000000028 RDI: ffff8800060e5a40
    [ 940.026915] RBP: ffff8800060e5a40 R08: 0000000000000000 R09: ffff88000075cc90
    [ 940.026915] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88000737fda0
    [ 940.026915] R13: 0000000000000000 R14: 0000000000002000 R15: ffff880005d3b580
    [ 940.026915] FS: 00007f163dc5e800(0000) GS:ffffffff81623000(0000) knlGS:0000000000000000
    [ 940.026915] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    [ 940.026915] CR2: 00000004032dc940 CR3: 0000000005c25000 CR4: 00000000000006f0
    [ 940.026915] Stack:
    [ 940.026915] ffff88000075cc4e ffffffff81694e90 ffff880000c30b38 0000000000000020
    [ 940.026915] 11000000523c4bac ffff88000737fdb4 0000000000000000 ffff880000c30800
    [ 940.026915] ffff880005d3b580 ffff880000c30b38 ffff8800060e5a40 0000000000000020
    [ 940.026915] Call Trace:
    [ 940.026915] [] ? inet6_csk_xmit+0xa4/0xc4
    [ 940.026915] [] ? l2tp_xmit_skb+0x503/0x55a [l2tp_core]
    [ 940.026915] [] ? pskb_expand_head+0x161/0x214
    [ 940.026915] [] ? pppol2tp_xmit+0xf2/0x143 [l2tp_ppp]
    [ 940.026915] [] ? ppp_channel_push+0x36/0x8b [ppp_generic]
    [ 940.026915] [] ? ppp_write+0xaf/0xc5 [ppp_generic]
    [ 940.026915] [] ? vfs_write+0xa2/0x106
    [ 940.026915] [] ? SyS_write+0x56/0x8a
    [ 940.026915] [] ? system_call_fastpath+0x16/0x1b
    [ 940.026915] Code: 00 49 8b 8f d8 00 00 00 66 83 7c 11 02 00 74 60 49
    8b 47 58 48 83 e0 fe 48 8b 80 18 01 00 00 48 85 c0 74 13 48 8b 80 78 02
    00 00 ff 40 28 41 8b 57 68 48 01 50 30 48 8b 54 24 08 49 c7 c1 51
    [ 940.026915] RIP [] ip6_xmit+0x276/0x326
    [ 940.026915] RSP
    [ 940.057945] ---[ end trace be8aba9a61c8b7f3 ]---
    [ 940.058583] Kernel panic - not syncing: Fatal exception in interrupt

    Signed-off-by: François CACHEREUL
    Signed-off-by: David S. Miller

    François Cachereul
     

03 Jul, 2013

2 commits

  • If L2TP data sequence numbers are enabled and reordering is not
    enabled, data reception stops if a packet is lost since the kernel
    waits for a sequence number that is never resent. (When reordering is
    enabled, data reception restarts when the reorder timeout expires.) If
    no reorder timeout is set, we should count the number of in-sequence
    packets after the out-of-sequence (OOS) condition is detected, and reset
    sequence number state after a number of such packets are received.

    For now, the number of in-sequence packets while in OOS state which
    cause the sequence number state to be reset is hard-coded to 5. This
    could be configurable later.

    Signed-off-by: James Chapman
    Signed-off-by: David S. Miller

    James Chapman
     
  • The L2TP datapath is not currently RFC-compliant when sequence numbers
    are used in L2TP data packets. According to the L2TP RFC, any received
    sequence number NR greater than or equal to the next expected NR is
    acceptable, where the "greater than or equal to" test is determined by
    the NR wrap point. This differs for L2TPv2 and L2TPv3, so add state in
    the session context to hold the max NR value and the NR window size in
    order to do the acceptable sequence number value check. These might be
    configurable later, but for now we derive it from the tunnel L2TP
    version, which determines the sequence number field size.

    Signed-off-by: James Chapman
    Signed-off-by: David S. Miller

    James Chapman
     

21 Mar, 2013

2 commits

  • If we postpone unhashing of l2tp sessions until the structure is freed, we
    risk:

    1. further packets arriving and getting queued while the pseudowire is being
    closed down
    2. the recv path hitting "scheduling while atomic" errors in the case that
    recv drops the last reference to a session and calls l2tp_session_free
    while in atomic context

    As such, l2tp sessions should be unhashed from l2tp_core data structures early
    in the teardown process prior to calling pseudowire close. For pseudowires
    like l2tp_ppp which have multiple shutdown codepaths, provide an unhash hook.

    Signed-off-by: Tom Parkin
    Signed-off-by: James Chapman
    Signed-off-by: David S. Miller

    Tom Parkin
     
  • l2tp's u64_stats writers were incorrectly synchronised, making it possible to
    deadlock a 64bit machine running a 32bit kernel simply by sending the l2tp
    code netlink commands while passing data through l2tp sessions.

    Previous discussion on netdev determined that alternative solutions such as
    spinlock writer synchronisation or per-cpu data would bring unjustified
    overhead, given that most users interested in high volume traffic will likely
    be running 64bit kernels on 64bit hardware.

    As such, this patch replaces l2tp's use of u64_stats with atomic_long_t,
    thereby avoiding the deadlock.

    Ref:
    http://marc.info/?l=linux-netdev&m=134029167910731&w=2
    http://marc.info/?l=linux-netdev&m=134079868111131&w=2

    Signed-off-by: Tom Parkin
    Signed-off-by: James Chapman
    Signed-off-by: David S. Miller

    Tom Parkin