1. 17 Nov, 2016 1 commit
    • Xin Long's avatar
      sctp: use new rhlist interface on sctp transport rhashtable · 7fda702f
      Xin Long authored
      
      
      Now sctp transport rhashtable uses hash(lport, dport, daddr) as the key
      to hash a node to one chain. If in one host thousands of assocs connect
      to one server with the same lport and different laddrs (although it's
      not a normal case), all the transports would be hashed into the same
      chain.
      
      It may cause to keep returning -EBUSY when inserting a new node, as the
      chain is too long and sctp inserts a transport node in a loop, which
      could even lead to system hangs there.
      
      The new rhlist interface works for this case that there are many nodes
      with the same key in one chain. It puts them into a list then makes this
      list be as a node of the chain.
      
      This patch is to replace rhashtable_ interface with rhltable_ interface.
      Since a chain would not be too long and it would not return -EBUSY with
      this fix when inserting a node, the reinsert loop is also removed here.
      Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
      Acked-by: default avatarNeil Horman <nhorman@tuxdriver.com>
      Acked-by: default avatarMarcelo Ricardo Leitner <marcelo.leitner@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      7fda702f
  2. 14 Nov, 2016 1 commit
  3. 07 Nov, 2016 1 commit
  4. 02 Nov, 2016 1 commit
  5. 31 Oct, 2016 3 commits
  6. 29 Oct, 2016 1 commit
  7. 26 Oct, 2016 1 commit
    • Xin Long's avatar
      sctp: fix the panic caused by route update · ecc515d7
      Xin Long authored
      Commit 7303a147 ("sctp: identify chunks that need to be fragmented
      at IP level") made the chunk be fragmented at IP level in the next round
      if it's size exceed PMTU.
      
      But there still is another case, PMTU can be updated if transport's dst
      expires and transport's pmtu_pending is set in sctp_packet_transmit. If
      the new PMTU is less than the chunk, the same issue with that commit can
      be triggered.
      
      So we should drop this packet and let it retransmit in another round
      where it would be fragmented at IP level.
      
      This patch is to fix it by checking the chunk size after PMTU may be
      updated and dropping this packet if it's size exceed PMTU.
      
      Fixes: 90017acc
      
       ("sctp: Add GSO support")
      Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
      Acked-by: default avatarNeil Horman <nhorman@txudriver.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      ecc515d7
  8. 23 Oct, 2016 1 commit
    • Jiri Slaby's avatar
      net: sctp, forbid negative length · a4b8e71b
      Jiri Slaby authored
      
      
      Most of getsockopt handlers in net/sctp/socket.c check len against
      sizeof some structure like:
              if (len < sizeof(int))
                      return -EINVAL;
      
      On the first look, the check seems to be correct. But since len is int
      and sizeof returns size_t, int gets promoted to unsigned size_t too. So
      the test returns false for negative lengths. Yes, (-1 < sizeof(long)) is
      false.
      
      Fix this in sctp by explicitly checking len < 0 before any getsockopt
      handler is called.
      
      Note that sctp_getsockopt_events already handled the negative case.
      Since we added the < 0 check elsewhere, this one can be removed.
      
      If not checked, this is the result:
      UBSAN: Undefined behaviour in ../mm/page_alloc.c:2722:19
      shift exponent 52 is too large for 32-bit type 'int'
      CPU: 1 PID: 24535 Comm: syz-executor Not tainted 4.8.1-0-syzkaller #1
      Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.1-0-gb3ef39f-prebuilt.qemu-project.org 04/01/2014
       0000000000000000 ffff88006d99f2a8 ffffffffb2f7bdea 0000000041b58ab3
       ffffffffb4363c14 ffffffffb2f7bcde ffff88006d99f2d0 ffff88006d99f270
       0000000000000000 0000000000000000 0000000000000034 ffffffffb5096422
      Call Trace:
       [<ffffffffb3051498>] ? __ubsan_handle_shift_out_of_bounds+0x29c/0x300
      ...
       [<ffffffffb273f0e4>] ? kmalloc_order+0x24/0x90
       [<ffffffffb27416a4>] ? kmalloc_order_trace+0x24/0x220
       [<ffffffffb2819a30>] ? __kmalloc+0x330/0x540
       [<ffffffffc18c25f4>] ? sctp_getsockopt_local_addrs+0x174/0xca0 [sctp]
       [<ffffffffc18d2bcd>] ? sctp_getsockopt+0x10d/0x1b0 [sctp]
       [<ffffffffb37c1219>] ? sock_common_getsockopt+0xb9/0x150
       [<ffffffffb37be2f5>] ? SyS_getsockopt+0x1a5/0x270
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      Cc: Vlad Yasevich <vyasevich@gmail.com>
      Cc: Neil Horman <nhorman@tuxdriver.com>
      Cc: "David S. Miller" <davem@davemloft.net>
      Cc: linux-sctp@vger.kernel.org
      Cc: netdev@vger.kernel.org
      Acked-by: default avatarNeil Horman <nhorman@tuxdriver.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      a4b8e71b
  9. 13 Oct, 2016 2 commits
  10. 30 Sep, 2016 5 commits
  11. 23 Sep, 2016 1 commit
  12. 22 Sep, 2016 2 commits
  13. 19 Sep, 2016 7 commits
    • Christophe Jaillet's avatar
      sctp: Remove some redundant code · e8bc8f9a
      Christophe Jaillet authored
      In commit 311b2177
      
       ("sctp: simplify sk_receive_queue locking"), a call
      to 'skb_queue_splice_tail_init()' has been made explicit. Previously it was
      hidden in 'sctp_skb_list_tail()'
      
      Now, the code around it looks redundant. The '_init()' part of
      'skb_queue_splice_tail_init()' should already do the same.
      Signed-off-by: default avatarChristophe JAILLET <christophe.jaillet@wanadoo.fr>
      Acked-by: default avatarMarcelo Ricardo Leitner <marcelo.leitner@gmail.com>
      Acked-by: default avatarNeil Horman <nhorman@tuxdriver.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e8bc8f9a
    • Xin Long's avatar
      sctp: not return ENOMEM err back in sctp_packet_transmit · 41001faf
      Xin Long authored
      
      
      As David and Marcelo's suggestion, ENOMEM err shouldn't return back to
      user in transmit path. Instead, sctp's retransmit would take care of
      the chunks that fail to send because of ENOMEM.
      
      This patch is only to do some release job when alloc_skb fails, not to
      return ENOMEM back any more.
      
      Besides, it also cleans up sctp_packet_transmit's err path, and fixes
      some issues in err path:
      
       - It didn't free the head skb in nomem: path.
       - No need to check nskb in no_route: path.
       - It should goto err: path if alloc_skb fails for head.
       - Not all the NOMEMs should free nskb.
      Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      41001faf
    • Xin Long's avatar
      sctp: make sctp_outq_flush/tail/uncork return void · 83dbc3d4
      Xin Long authored
      
      
      sctp_outq_flush return value is meaningless now, this patch is
      to make sctp_outq_flush return void, as well as sctp_outq_fail
      and sctp_outq_uncork.
      Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      83dbc3d4
    • Xin Long's avatar
      sctp: save transmit error to sk_err in sctp_outq_flush · 64519440
      Xin Long authored
      
      
      Every time when sctp calls sctp_outq_flush, it sends out the chunks of
      control queue, retransmit queue and data queue. Even if some trunks are
      failed to transmit, it still has to flush all the transports, as it's
      the only chance to clean that transmit_list.
      
      So the latest transmit error here should be returned back. This transmit
      error is an internal error of sctp stack.
      
      I checked all the places where it uses the transmit error (the return
      value of sctp_outq_flush), most of them are actually just save it to
      sk_err.
      
      Except for sctp_assoc/endpoint_bh_rcv, they will drop the chunk if
      it's failed to send a REPLY, which is actually incorrect, as we can't
      be sure the error that sctp_outq_flush returns is from sending that
      REPLY.
      
      So it's meaningless for sctp_outq_flush to return error back.
      
      This patch is to save transmit error to sk_err in sctp_outq_flush, the
      new error can update the old value. Eventually, sctp_wait_for_* would
      check for it.
      Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      64519440
    • Xin Long's avatar
      sctp: free msg->chunks when sctp_primitive_SEND return err · b61c654f
      Xin Long authored
      Last patch "sctp: do not return the transmit err back to sctp_sendmsg"
      made sctp_primitive_SEND return err only when asoc state is unavailable.
      In this case, chunks are not enqueued, they have no chance to be freed if
      we don't take care of them later.
      
      This Patch is actually to revert commit 1cd4d5c4 ("sctp: remove the
      unused sctp_datamsg_free()"), commit 69b5777f ("sctp: hold the chunks
      only after the chunk is enqueued in outq") and commit 8b570dc9 ("sctp:
      only drop the reference on the datamsg after sending a msg"), to use
      sctp_datamsg_free to free the chunks of current msg.
      
      Fixes: 8b570dc9
      
       ("sctp: only drop the reference on the datamsg after sending a msg")
      Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      b61c654f
    • Xin Long's avatar
      sctp: do not return the transmit err back to sctp_sendmsg · 66388f2c
      Xin Long authored
      Once a chunk is enqueued successfully, sctp queues can take care of it.
      Even if it is failed to transmit (like because of nomem), it should be
      put into retransmit queue.
      
      If sctp report this error to users, it confuses them, they may resend
      that msg, but actually in kernel sctp stack is in charge of retransmit
      it already.
      
      Besides, this error probably is not from the failure of transmitting
      current msg, but transmitting or retransmitting another msg's chunks,
      as sctp_outq_flush just tries to send out all transports' chunks.
      
      This patch is to make sctp_cmd_send_msg return avoid, and not return the
      transmit err back to sctp_sendmsg
      
      Fixes: 8b570dc9
      
       ("sctp: only drop the reference on the datamsg after sending a msg")
      Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      66388f2c
    • Xin Long's avatar
      sctp: remove the unnecessary state check in sctp_outq_tail · 2c89791e
      Xin Long authored
      
      
      Data Chunks are only sent by sctp_primitive_SEND, in which sctp checks
      the asoc's state through statetable before calling sctp_outq_tail. So
      there's no need to check the asoc's state again in sctp_outq_tail.
      
      Besides, sctp_do_sm is protected by lock_sock, even if sending msg is
      interrupted by timer events, the event's processes still need to acquire
      lock_sock first. It means no others CMDs can be enqueue into side effect
      list before CMD_SEND_MSG to change asoc->state, so it's safe to remove it.
      
      This patch is to remove redundant asoc->state check from sctp_outq_tail.
      Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      2c89791e
  14. 13 Sep, 2016 1 commit
    • Xin Long's avatar
      sctp: hold the transport before using it in sctp_hash_cmp · 715f5552
      Xin Long authored
      Since commit 4f008781 ("sctp: apply rhashtable api to send/recv
      path"), sctp uses transport rhashtable with .obj_cmpfn sctp_hash_cmp,
      in which it compares the members of the transport with the rhashtable
      args to check if it's the right transport.
      
      But sctp uses the transport without holding it in sctp_hash_cmp, it can
      cause a use-after-free panic. As after it gets transport from hashtable,
      another CPU may close the sk and free the asoc. In sctp_association_free,
      it frees all the transports, meanwhile, the assoc's refcnt may be reduced
      to 0, assoc can be destroyed by sctp_association_destroy.
      
      So after that, transport->assoc is actually an unavailable memory address
      in sctp_hash_cmp. Although sctp_hash_cmp is under rcu_read_lock, it still
      can not avoid this, as assoc is not freed by RCU.
      
      This patch is to hold the transport before checking it's members with
      sctp_transport_hold, in which it checks the refcnt first, holds it if
      it's not 0.
      
      Fixes: 4f008781
      
       ("sctp: apply rhashtable api to send/recv path")
      Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
      Acked-by: default avatarMarcelo Ricardo Leitner <marcelo.leitner@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      715f5552
  15. 11 Sep, 2016 1 commit
  16. 10 Sep, 2016 1 commit
    • Marcelo Ricardo Leitner's avatar
      sctp: identify chunks that need to be fragmented at IP level · 7303a147
      Marcelo Ricardo Leitner authored
      
      
      Previously, without GSO, it was easy to identify it: if the chunk didn't
      fit and there was no data chunk in the packet yet, we could fragment at
      IP level. So if there was an auth chunk and we were bundling a big data
      chunk, it would fragment regardless of the size of the auth chunk. This
      also works for the context of PMTU reductions.
      
      But with GSO, we cannot distinguish such PMTU events anymore, as the
      packet is allowed to exceed PMTU.
      
      So we need another check: to ensure that the chunk that we are adding,
      actually fits the current PMTU. If it doesn't, trigger a flush and let
      it be fragmented at IP level in the next round.
      Signed-off-by: default avatarMarcelo Ricardo Leitner <marcelo.leitner@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      7303a147
  17. 08 Sep, 2016 1 commit
  18. 24 Aug, 2016 1 commit
  19. 20 Aug, 2016 1 commit
    • Marcelo Ricardo Leitner's avatar
      sctp: linearize early if it's not GSO · 4c2f2454
      Marcelo Ricardo Leitner authored
      Because otherwise when crc computation is still needed it's way more
      expensive than on a linear buffer to the point that it affects
      performance.
      
      It's so expensive that netperf test gives a perf output as below:
      
      Overhead  Command         Shared Object       Symbol
        18,62%  netserver       [kernel.vmlinux]    [k] crc32_generic_shift
         2,57%  netserver       [kernel.vmlinux]    [k] __pskb_pull_tail
         1,94%  netserver       [kernel.vmlinux]    [k] fib_table_lookup
         1,90%  netserver       [kernel.vmlinux]    [k] copy_user_enhanced_fast_string
         1,66%  swapper         [kernel.vmlinux]    [k] intel_idle
         1,63%  netserver       [kernel.vmlinux]    [k] _raw_spin_lock
         1,59%  netserver       [sctp]              [k] sctp_packet_transmit
         1,55%  netserver       [kernel.vmlinux]    [k] memcpy_erms
         1,42%  netserver       [sctp]              [k] sctp_rcv
      
      # netperf -H 192.168.10.1 -l 10 -t SCTP_STREAM -cC -- -m 12000
      SCTP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.10.1 () port 0 AF_INET
      Recv   Send    Send                          Utilization       Service Demand
      Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
      Size   Size    Size     Time     Throughput  local    remote   local   remote
      bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   us/KB
      
      212992 212992  12000    10.00      3016.42   2.88     3.78     1.874   2.462
      
      After patch:
      Overhead  Command         Shared Object      Symbol
         2,75%  netserver       [kernel.vmlinux]   [k] memcpy_erms
         2,63%  netserver       [kernel.vmlinux]   [k] copy_user_enhanced_fast_string
         2,39%  netserver       [kernel.vmlinux]   [k] fib_table_lookup
         2,04%  netserver       [kernel.vmlinux]   [k] __pskb_pull_tail
         1,91%  netserver       [kernel.vmlinux]   [k] _raw_spin_lock
         1,91%  netserver       [sctp]             [k] sctp_packet_transmit
         1,72%  netserver       [mlx4_en]          [k] mlx4_en_process_rx_cq
         1,68%  netserver       [sctp]             [k] sctp_rcv
      
      # netperf -H 192.168.10.1 -l 10 -t SCTP_STREAM -cC -- -m 12000
      SCTP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.10.1 () port 0 AF_INET
      Recv   Send    Send                          Utilization       Service Demand
      Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
      Size   Size    Size     Time     Throughput  local    remote   local   remote
      bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   us/KB
      
      212992 212992  12000    10.00      3681.77   3.83     3.46     2.045   1.849
      
      Fixes: 3acb50c1
      
       ("sctp: delay as much as possible skb_linearize")
      Signed-off-by: default avatarMarcelo Ricardo Leitner <marcelo.leitner@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      4c2f2454
  20. 13 Aug, 2016 1 commit
    • Vegard Nossum's avatar
      net/sctp: always initialise sctp_ht_iter::start_fail · 54236ab0
      Vegard Nossum authored
      sctp_transport_seq_start() does not currently clear iter->start_fail on
      success, but relies on it being zero when it is allocated (by
      seq_open_net()).
      
      This can be a problem in the following sequence:
      
          open() // allocates iter (and implicitly sets iter->start_fail = 0)
          read()
           - iter->start() // fails and sets iter->start_fail = 1
           - iter->stop() // doesn't call sctp_transport_walk_stop() (correct)
          read() again
           - iter->start() // succeeds, but doesn't change iter->start_fail
           - iter->stop() // doesn't call sctp_transport_walk_stop() (wrong)
      
      We should initialize sctp_ht_iter::start_fail to zero if ->start()
      succeeds, otherwise it's possible that we leave an old value of 1 there,
      which will cause ->stop() to not call sctp_transport_walk_stop(), which
      causes all sorts of problems like not calling rcu_read_unlock() (and
      preempt_enable()), eventually leading to more warnings like this:
      
          BUG: sleeping function called from invalid context at mm/slab.h:388
          in_atomic(): 0, irqs_disabled(): 0, pid: 16551, name: trinity-c2
          Preemption disabled at:[<ffffffff819bceb6>] rhashtable_walk_start+0x46/0x150
      
           [<ffffffff81149abb>] preempt_count_add+0x1fb/0x280
           [<ffffffff83295892>] _raw_spin_lock+0x12/0x40
           [<ffffffff819bceb6>] rhashtable_walk_start+0x46/0x150
           [<ffffffff82ec665f>] sctp_transport_walk_start+0x2f/0x60
           [<ffffffff82edda1d>] sctp_transport_seq_start+0x4d/0x150
           [<ffffffff81439e50>] traverse+0x170/0x850
           [<ffffffff8143aeec>] seq_read+0x7cc/0x1180
           [<ffffffff814f996c>] proc_reg_read+0xbc/0x180
           [<ffffffff813d0384>] do_loop_readv_writev+0x134/0x210
           [<ffffffff813d2a95>] do_readv_writev+0x565/0x660
           [<ffffffff813d6857>] vfs_readv+0x67/0xa0
           [<ffffffff813d6c16>] do_preadv+0x126/0x170
           [<ffffffff813d710c>] SyS_preadv+0xc/0x10
           [<ffffffff8100334c>] do_syscall_64+0x19c/0x410
           [<ffffffff83296225>] return_from_SYSCALL_64+0x0/0x6a
           [<ffffffffffffffff>] 0xffffffffffffffff
      
      Notice that this is a subtly different stacktrace from the one in commit
      5fc382d8
      
       ("net/sctp: terminate rhashtable walk correctly").
      
      Cc: Xin Long <lucien.xin@gmail.com>
      Cc: Herbert Xu <herbert@gondor.apana.org.au>
      Cc: Eric W. Biederman <ebiederm@xmission.com>
      Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
      Signed-off-by: default avatarVegard Nossum <vegard.nossum@oracle.com>
      Acked-By: default avatarNeil Horman <nhorman@tuxdriver.com>
      Acked-by: default avatarMarcelo Ricardo Leitner <marcelo.leitner@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      54236ab0
  21. 08 Aug, 2016 3 commits
  22. 31 Jul, 2016 3 commits