1. 12 8月, 2018 3 次提交
    • G
      l2tp: simplify pppol2tp_ioctl() · bdd0292f
      Guillaume Nault 提交于
      * Drop test on 'sk': sock->sk cannot be NULL, or pppox_ioctl() could
          not have called us.
      
        * Drop test on 'SOCK_DEAD' state: if this flag was set, the socket
          would be in the process of being released and no ioctl could be
          running anymore.
      
        * Drop test on 'PPPOX_*' state: we depend on ->sk_user_data to get
          the session structure. If it is non-NULL, then the socket is
          connected. Testing for PPPOX_* is redundant.
      
        * Retrieve session using ->sk_user_data directly, instead of going
          through pppol2tp_sock_to_session(). This avoids grabbing a useless
          reference on the socket.
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      bdd0292f
    • G
      l2tp: split l2tp_session_get() · 01e28b92
      Guillaume Nault 提交于
      l2tp_session_get() is used for two different purposes. If 'tunnel' is
      NULL, the session is searched globally in the supplied network
      namespace. Otherwise it is searched exclusively in the tunnel context.
      
      Callers always know the context in which they need to search the
      session. But some of them do provide both a namespace and a tunnel,
      making the semantic of the call unclear.
      
      This patch defines l2tp_tunnel_get_session() for lookups done in a
      tunnel and restricts l2tp_session_get() to namespace searches.
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      01e28b92
    • G
      l2tp: define l2tp_tunnel_uses_xfrm() · d6a61ec9
      Guillaume Nault 提交于
      Use helper function to figure out if a tunnel is using ipsec.
      Also, avoid accessing ->sk_policy directly since it's RCU protected.
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      d6a61ec9
  2. 04 8月, 2018 4 次提交
    • G
      l2tp: fix missing refcount drop in pppol2tp_tunnel_ioctl() · f664e37d
      Guillaume Nault 提交于
      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: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      f664e37d
    • G
      l2tp: ignore L2TP_ATTR_MTU · e9697e2e
      Guillaume Nault 提交于
      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: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      e9697e2e
    • G
      l2tp: simplify MTU handling in l2tp_ppp · 789141b2
      Guillaume Nault 提交于
      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: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      789141b2
    • G
      l2tp: define l2tp_tunnel_dst_mtu() · 1f5cd2a0
      Guillaume Nault 提交于
      Consolidate retrieval of tunnel's socket mtu in order to simplify
      l2tp_eth and l2tp_ppp a bit.
      Signed-off-by: NGuillaume Nault <g.nault@alphalink.fr>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      1f5cd2a0
  3. 28 7月, 2018 4 次提交
  4. 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
  5. 25 7月, 2018 1 次提交
  6. 07 7月, 2018 2 次提交
  7. 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
  8. 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
  9. 26 6月, 2018 7 次提交
  10. 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
  11. 15 6月, 2018 4 次提交
  12. 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
  13. 26 5月, 2018 1 次提交
  14. 16 5月, 2018 1 次提交
  15. 27 4月, 2018 1 次提交
  16. 24 4月, 2018 1 次提交
  17. 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
  18. 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
  19. 12 4月, 2018 1 次提交