20 Dec, 2017

1 commit

  • [ Upstream commit 4dc12ffeaeac939097a3f55c881d3dc3523dff0c ]

    l2tp_tunnel_delete does not return anything since commit 62b982eeb458
    ("l2tp: fix race condition in l2tp_tunnel_delete"). But call sites of
    l2tp_tunnel_delete still do casts to void to avoid unused return value
    warnings.

    Kill these now useless casts.

    Signed-off-by: Jiri Slaby
    Cc: Sabrina Dubroca
    Cc: Guillaume Nault
    Cc: David S. Miller
    Cc: netdev@vger.kernel.org
    Acked-by: Guillaume Nault
    Signed-off-by: David S. Miller
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Jiri Slaby
     

05 Nov, 2017

1 commit

  • Using l2tp_tunnel_find() in l2tp_ip_recv() is wrong for two reasons:

    * It doesn't take a reference on the returned tunnel, which makes the
    call racy wrt. concurrent tunnel deletion.

    * The lookup is only based on the tunnel identifier, so it can return
    a tunnel that doesn't match the packet's addresses or protocol.

    For example, a packet sent to an L2TPv3 over IPv6 tunnel can be
    delivered to an L2TPv2 over UDPv4 tunnel. This is worse than a simple
    cross-talk: when delivering the packet to an L2TP over UDP tunnel, the
    corresponding socket is UDP, where ->sk_backlog_rcv() is NULL. Calling
    sk_receive_skb() will then crash the kernel by trying to execute this
    callback.

    And l2tp_tunnel_find() isn't even needed here. __l2tp_ip_bind_lookup()
    properly checks the socket binding and connection settings. It was used
    as a fallback mechanism for finding tunnels that didn't have their data
    path registered yet. But it's not limited to this case and can be used
    to replace l2tp_tunnel_find() in the general case.

    Fix l2tp_ip6 in the same way.

    Fixes: 0d76751fad77 ("l2tp: Add L2TPv3 IP encapsulation (no UDP) support")
    Fixes: a32e0eec7042 ("l2tp: introduce L2TPv3 IP encapsulation support for IPv6")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

03 Nov, 2017

1 commit

  • …el/git/gregkh/driver-core

    Pull initial SPDX identifiers from Greg KH:
    "License cleanup: add SPDX license identifiers to some files

    Many source files in the tree are missing licensing information, which
    makes it harder for compliance tools to determine the correct license.

    By default all files without license information are under the default
    license of the kernel, which is GPL version 2.

    Update the files which contain no license information with the
    'GPL-2.0' SPDX license identifier. The SPDX identifier is a legally
    binding shorthand, which can be used instead of the full boiler plate
    text.

    This patch is based on work done by Thomas Gleixner and Kate Stewart
    and Philippe Ombredanne.

    How this work was done:

    Patches were generated and checked against linux-4.14-rc6 for a subset
    of the use cases:

    - file had no licensing information it it.

    - file was a */uapi/* one with no licensing information in it,

    - file was a */uapi/* one with existing licensing information,

    Further patches will be generated in subsequent months to fix up cases
    where non-standard license headers were used, and references to
    license had to be inferred by heuristics based on keywords.

    The analysis to determine which SPDX License Identifier to be applied
    to a file was done in a spreadsheet of side by side results from of
    the output of two independent scanners (ScanCode & Windriver)
    producing SPDX tag:value files created by Philippe Ombredanne.
    Philippe prepared the base worksheet, and did an initial spot review
    of a few 1000 files.

    The 4.13 kernel was the starting point of the analysis with 60,537
    files assessed. Kate Stewart did a file by file comparison of the
    scanner results in the spreadsheet to determine which SPDX license
    identifier(s) to be applied to the file. She confirmed any
    determination that was not immediately clear with lawyers working with
    the Linux Foundation.

    Criteria used to select files for SPDX license identifier tagging was:

    - Files considered eligible had to be source code files.

    - Make and config files were included as candidates if they contained
    >5 lines of source

    - File already had some variant of a license header in it (even if <5
    lines).

    All documentation files were explicitly excluded.

    The following heuristics were used to determine which SPDX license
    identifiers to apply.

    - when both scanners couldn't find any license traces, file was
    considered to have no license information in it, and the top level
    COPYING file license applied.

    For non */uapi/* files that summary was:

    SPDX license identifier # files
    ---------------------------------------------------|-------
    GPL-2.0 11139

    and resulted in the first patch in this series.

    If that file was a */uapi/* path one, it was "GPL-2.0 WITH
    Linux-syscall-note" otherwise it was "GPL-2.0". Results of that
    was:

    SPDX license identifier # files
    ---------------------------------------------------|-------
    GPL-2.0 WITH Linux-syscall-note 930

    and resulted in the second patch in this series.

    - if a file had some form of licensing information in it, and was one
    of the */uapi/* ones, it was denoted with the Linux-syscall-note if
    any GPL family license was found in the file or had no licensing in
    it (per prior point). Results summary:

    SPDX license identifier # files
    ---------------------------------------------------|------
    GPL-2.0 WITH Linux-syscall-note 270
    GPL-2.0+ WITH Linux-syscall-note 169
    ((GPL-2.0 WITH Linux-syscall-note) OR BSD-2-Clause) 21
    ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) 17
    LGPL-2.1+ WITH Linux-syscall-note 15
    GPL-1.0+ WITH Linux-syscall-note 14
    ((GPL-2.0+ WITH Linux-syscall-note) OR BSD-3-Clause) 5
    LGPL-2.0+ WITH Linux-syscall-note 4
    LGPL-2.1 WITH Linux-syscall-note 3
    ((GPL-2.0 WITH Linux-syscall-note) OR MIT) 3
    ((GPL-2.0 WITH Linux-syscall-note) AND MIT) 1

    and that resulted in the third patch in this series.

    - when the two scanners agreed on the detected license(s), that
    became the concluded license(s).

    - when there was disagreement between the two scanners (one detected
    a license but the other didn't, or they both detected different
    licenses) a manual inspection of the file occurred.

    - In most cases a manual inspection of the information in the file
    resulted in a clear resolution of the license that should apply
    (and which scanner probably needed to revisit its heuristics).

    - When it was not immediately clear, the license identifier was
    confirmed with lawyers working with the Linux Foundation.

    - If there was any question as to the appropriate license identifier,
    the file was flagged for further research and to be revisited later
    in time.

    In total, over 70 hours of logged manual review was done on the
    spreadsheet to determine the SPDX license identifiers to apply to the
    source files by Kate, Philippe, Thomas and, in some cases,
    confirmation by lawyers working with the Linux Foundation.

    Kate also obtained a third independent scan of the 4.13 code base from
    FOSSology, and compared selected files where the other two scanners
    disagreed against that SPDX file, to see if there was new insights.
    The Windriver scanner is based on an older version of FOSSology in
    part, so they are related.

    Thomas did random spot checks in about 500 files from the spreadsheets
    for the uapi headers and agreed with SPDX license identifier in the
    files he inspected. For the non-uapi files Thomas did random spot
    checks in about 15000 files.

    In initial set of patches against 4.14-rc6, 3 files were found to have
    copy/paste license identifier errors, and have been fixed to reflect
    the correct identifier.

    Additionally Philippe spent 10 hours this week doing a detailed manual
    inspection and review of the 12,461 patched files from the initial
    patch version early this week with:

    - a full scancode scan run, collecting the matched texts, detected
    license ids and scores

    - reviewing anything where there was a license detected (about 500+
    files) to ensure that the applied SPDX license was correct

    - reviewing anything where there was no detection but the patch
    license was not GPL-2.0 WITH Linux-syscall-note to ensure that the
    applied SPDX license was correct

    This produced a worksheet with 20 files needing minor correction. This
    worksheet was then exported into 3 different .csv files for the
    different types of files to be modified.

    These .csv files were then reviewed by Greg. Thomas wrote a script to
    parse the csv files and add the proper SPDX tag to the file, in the
    format that the file expected. This script was further refined by Greg
    based on the output to detect more types of files automatically and to
    distinguish between header and source .c files (which need different
    comment types.) Finally Greg ran the script using the .csv files to
    generate the patches.

    Reviewed-by: Kate Stewart <kstewart@linuxfoundation.org>
    Reviewed-by: Philippe Ombredanne <pombredanne@nexb.com>
    Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>"

    * tag 'spdx_identifiers-4.14-rc8' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core:
    License cleanup: add SPDX license identifier to uapi header files with a license
    License cleanup: add SPDX license identifier to uapi header files with no license
    License cleanup: add SPDX GPL-2.0 license identifier to files with no license

    Linus Torvalds
     

02 Nov, 2017

1 commit

  • Many source files in the tree are missing licensing information, which
    makes it harder for compliance tools to determine the correct license.

    By default all files without license information are under the default
    license of the kernel, which is GPL version 2.

    Update the files which contain no license information with the 'GPL-2.0'
    SPDX license identifier. The SPDX identifier is a legally binding
    shorthand, which can be used instead of the full boiler plate text.

    This patch is based on work done by Thomas Gleixner and Kate Stewart and
    Philippe Ombredanne.

    How this work was done:

    Patches were generated and checked against linux-4.14-rc6 for a subset of
    the use cases:
    - file had no licensing information it it.
    - file was a */uapi/* one with no licensing information in it,
    - file was a */uapi/* one with existing licensing information,

    Further patches will be generated in subsequent months to fix up cases
    where non-standard license headers were used, and references to license
    had to be inferred by heuristics based on keywords.

    The analysis to determine which SPDX License Identifier to be applied to
    a file was done in a spreadsheet of side by side results from of the
    output of two independent scanners (ScanCode & Windriver) producing SPDX
    tag:value files created by Philippe Ombredanne. Philippe prepared the
    base worksheet, and did an initial spot review of a few 1000 files.

    The 4.13 kernel was the starting point of the analysis with 60,537 files
    assessed. Kate Stewart did a file by file comparison of the scanner
    results in the spreadsheet to determine which SPDX license identifier(s)
    to be applied to the file. She confirmed any determination that was not
    immediately clear with lawyers working with the Linux Foundation.

    Criteria used to select files for SPDX license identifier tagging was:
    - Files considered eligible had to be source code files.
    - Make and config files were included as candidates if they contained >5
    lines of source
    - File already had some variant of a license header in it (even if
    Reviewed-by: Philippe Ombredanne
    Reviewed-by: Thomas Gleixner
    Signed-off-by: Greg Kroah-Hartman

    Greg Kroah-Hartman
     

31 Oct, 2017

1 commit

  • Use l2tp_tunnel_get() in pppol2tp_connect() to ensure the tunnel isn't
    going to disappear while processing the rest of the function.

    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
     

15 Oct, 2017

1 commit

  • When pppol2tp_session_ioctl() is called by pppol2tp_tunnel_ioctl(),
    the session may be unconnected. That is, it was created by
    pppol2tp_session_create() and hasn't been connected with
    pppol2tp_connect(). In this case, ps->sock is NULL, so we need to check
    for this case in order to avoid dereferencing a NULL pointer.

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

    Guillaume Nault
     

02 Oct, 2017

1 commit

  • The l2tp_eth module crashes if its netlink callbacks are run when the
    pernet data aren't initialised.

    We should normally register_pernet_device() before the genl callbacks.
    However, the pernet data only maintain a list of l2tpeth interfaces,
    and this list is never used. So let's just drop pernet handling
    instead.

    Fixes: d9e31d17ceba ("l2tp: Add L2TP ethernet pseudowire support")
    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

2 commits

  • 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
     
  • If l2tp_tunnel_delete() or l2tp_tunnel_closeall() deletes a session
    right after pppol2tp_release() orphaned its socket, then the 'sock'
    variable of the pppol2tp_session_close() callback is NULL. Yet the
    session is still used by pppol2tp_release().

    Therefore we need to take an extra reference in any case, to prevent
    l2tp_tunnel_delete() or l2tp_tunnel_closeall() from freeing the session.

    Since the pppol2tp_session_close() callback is only set if the session
    is associated to a PPPOL2TP socket and that both l2tp_tunnel_delete()
    and l2tp_tunnel_closeall() hold the PPPOL2TP socket before calling
    pppol2tp_session_close(), we're sure that pppol2tp_session_close() and
    pppol2tp_session_destruct() are paired and called in the right order.
    So the reference taken by the former will be released by the later.

    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

6 commits

  • Use l2tp_tunnel_get() to retrieve tunnel, so that it can't go away on
    us. Otherwise l2tp_tunnel_destruct() might release the last reference
    count concurrently, thus freeing the tunnel while we're using it.

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

    Guillaume Nault
     
  • Use l2tp_tunnel_get() instead of l2tp_tunnel_find() so that we get
    a reference on the tunnel, preventing l2tp_tunnel_destruct() from
    freeing it from under us.

    Also move l2tp_tunnel_get() below nlmsg_new() so that we only take
    the reference when needed.

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

    Guillaume Nault
     
  • We need to make sure the tunnel is not going to be destroyed by
    l2tp_tunnel_destruct() concurrently.

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

    Guillaume Nault
     
  • l2tp_nl_cmd_tunnel_delete() needs to take a reference on the tunnel, to
    prevent it from being concurrently freed by l2tp_tunnel_destruct().

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

    Guillaume Nault
     
  • 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
     
  • Sessions must be fully initialised before calling
    l2tp_session_add_to_tunnel(). Otherwise, there's a short time frame
    where partially initialised sessions can be accessed by external users.

    Fixes: dbdbc73b4478 ("l2tp: fix duplicate session creation")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

05 Jul, 2017

2 commits


01 Jul, 2017

1 commit

  • refcount_t type and corresponding API should be
    used instead of atomic_t when the variable is used as
    a reference counter. This allows to avoid accidental
    refcounter overflows that might lead to use-after-free
    situations.

    This patch uses refcount_inc_not_zero() instead of
    atomic_inc_not_zero_hint() due to absense of a _hint()
    version of refcount API. If the hint() version must
    be used, we might need to revisit API.

    Signed-off-by: Elena Reshetova
    Signed-off-by: Hans Liljestrand
    Signed-off-by: Kees Cook
    Signed-off-by: David Windsor
    Signed-off-by: David S. Miller

    Reshetova, Elena
     

11 Jun, 2017

1 commit

  • This fixes a counter problem on 32bit systems:
    When the rx_bytes counter reached 2 GiB, it jumpd to (2^64 Bytes - 2GiB) Bytes.

    rtnl_link_stats64 has __u64 type and atomic_long_read returns
    atomic_long_t which is signed. Due to the conversation
    we get an incorrect value on 32bit systems if the MSB of
    the atomic_long_t value is set.

    CC: Tom Parkin
    Fixes: 7b7c0719cd7a ("l2tp: avoid deadlock in l2tp stats update")
    Signed-off-by: Dominik Heidler
    Signed-off-by: David S. Miller

    Dominik Heidler
     

08 Jun, 2017

1 commit

  • Network devices can allocate reasources and private memory using
    netdev_ops->ndo_init(). However, the release of these resources
    can occur in one of two different places.

    Either netdev_ops->ndo_uninit() or netdev->destructor().

    The decision of which operation frees the resources depends upon
    whether it is necessary for all netdev refs to be released before it
    is safe to perform the freeing.

    netdev_ops->ndo_uninit() presumably can occur right after the
    NETDEV_UNREGISTER notifier completes and the unicast and multicast
    address lists are flushed.

    netdev->destructor(), on the other hand, does not run until the
    netdev references all go away.

    Further complicating the situation is that netdev->destructor()
    almost universally does also a free_netdev().

    This creates a problem for the logic in register_netdevice().
    Because all callers of register_netdevice() manage the freeing
    of the netdev, and invoke free_netdev(dev) if register_netdevice()
    fails.

    If netdev_ops->ndo_init() succeeds, but something else fails inside
    of register_netdevice(), it does call ndo_ops->ndo_uninit(). But
    it is not able to invoke netdev->destructor().

    This is because netdev->destructor() will do a free_netdev() and
    then the caller of register_netdevice() will do the same.

    However, this means that the resources that would normally be released
    by netdev->destructor() will not be.

    Over the years drivers have added local hacks to deal with this, by
    invoking their destructor parts by hand when register_netdevice()
    fails.

    Many drivers do not try to deal with this, and instead we have leaks.

    Let's close this hole by formalizing the distinction between what
    private things need to be freed up by netdev->destructor() and whether
    the driver needs unregister_netdevice() to perform the free_netdev().

    netdev->priv_destructor() performs all actions to free up the private
    resources that used to be freed by netdev->destructor(), except for
    free_netdev().

    netdev->needs_free_netdev is a boolean that indicates whether
    free_netdev() should be done at the end of unregister_netdevice().

    Now, register_netdevice() can sanely release all resources after
    ndo_ops->ndo_init() succeeds, by invoking both ndo_ops->ndo_uninit()
    and netdev->priv_destructor().

    And at the end of unregister_netdevice(), we invoke
    netdev->priv_destructor() and optionally call free_netdev().

    Signed-off-by: David S. Miller

    David S. Miller
     

28 Apr, 2017

1 commit


25 Apr, 2017

2 commits


18 Apr, 2017

1 commit

  • The MTU overhead calculation in L2TP device set-up
    merged via commit b784e7ebfce8cfb16c6f95e14e8532d0768ab7ff
    needs to be adjusted to lock the tunnel socket while
    referencing the sub-data structures to derive the
    socket's IP overhead.

    Reported-by: Guillaume Nault
    Tested-by: Guillaume Nault
    Signed-off-by: R. Parameswaran
    Signed-off-by: David S. Miller

    R. Parameswaran
     

16 Apr, 2017

1 commit


12 Apr, 2017

4 commits


08 Apr, 2017

2 commits

  • pppol2tp_getsockopt() doesn't take into account the error code returned
    by pppol2tp_tunnel_getsockopt() or pppol2tp_session_getsockopt(). If
    error occurs there, pppol2tp_getsockopt() continues unconditionally and
    reports erroneous values.

    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
     
  • pppol2tp_setsockopt() unconditionally overwrites the error value
    returned by pppol2tp_tunnel_setsockopt() or
    pppol2tp_session_setsockopt(), thus hiding errors from userspace.

    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
     

07 Apr, 2017

1 commit

  • Existing L2TP kernel code does not derive the optimal MTU for Ethernet
    pseudowires and instead leaves this to a userspace L2TP daemon or
    operator. If an MTU is not specified, the existing kernel code chooses
    an MTU that does not take account of all tunnel header overheads, which
    can lead to unwanted IP fragmentation. When L2TP is used without a
    control plane (userspace daemon), we would prefer that the kernel does a
    better job of choosing a default pseudowire MTU, taking account of all
    tunnel header overheads, including IP header options, if any. This patch
    addresses this.

    Change-set here uses the new kernel function, kernel_sock_ip_overhead(),
    to factor the outer IP overhead on the L2TP tunnel socket (including
    IP Options, if any) when calculating the default MTU for an Ethernet
    pseudowire, along with consideration of the inner Ethernet header.

    Signed-off-by: R. Parameswaran
    Signed-off-by: David S. Miller

    R. Parameswaran
     

05 Apr, 2017

2 commits

  • PPP pseudo-wire type is 7 (11 is L2TP_PWTYPE_IP).

    Fixes: f1f39f911027 ("l2tp: auto load type modules")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     
  • 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

3 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
     
  • l2tp_session_find() doesn't take any reference on the returned session.
    Therefore, the session may disappear while sending the notification.

    Use l2tp_session_get() instead and decrement session's refcount once
    the notification is sent.

    Fixes: 33f72e6f0c67 ("l2tp : multicast notification to the registered listeners")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     
  • l2tp_session_create() relies on its caller for checking for duplicate
    sessions. This is racy since a session can be concurrently inserted
    after the caller's verification.

    Fix this by letting l2tp_session_create() verify sessions uniqueness
    upon insertion. Callers need to be adapted to check for
    l2tp_session_create()'s return code instead of calling
    l2tp_session_find().

    pppol2tp_connect() is a bit special because it has to work on existing
    sessions (if they're not connected) or to create a new session if none
    is found. When acting on a preexisting session, a reference must be
    held or it could go away on us. So we have to use l2tp_session_get()
    instead of l2tp_session_find() and drop the reference before exiting.

    Fixes: d9e31d17ceba ("l2tp: Add L2TP ethernet pseudowire support")
    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