1. 22 Mar, 2018 16 commits
  2. 21 Mar, 2018 12 commits
    • David S. Miller's avatar
      Merge branch 'net-sched-action-idr-leak' · ba9a1908
      David S. Miller authored
      Davide Caratti says:
      
      ====================
      fix idr leak in actions
      
      This series fixes situations where a temporary failure to install a TC
      action results in the permanent impossibility to reuse the configured
      value of 'index'.
      
      Thanks to Cong Wang for the initial review.
      
      v2: fix build error in act_ipt.c, reported by kbuild test robot
      ====================
      Acked-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      ba9a1908
    • Davide Caratti's avatar
      net/sched: fix idr leak in the error path of tcf_skbmod_init() · f29cdfbe
      Davide Caratti authored
      tcf_skbmod_init() can fail after the idr has been successfully reserved.
      When this happens, every subsequent attempt to configure skbmod rules
      using the same idr value will systematically fail with -ENOSPC, unless
      the first attempt was done using the 'replace' keyword:
      
       # tc action add action skbmod swap mac index 100
       RTNETLINK answers: Cannot allocate memory
       We have an error talking to the kernel
       # tc action add action skbmod swap mac index 100
       RTNETLINK answers: No space left on device
       We have an error talking to the kernel
       # tc action add action skbmod swap mac index 100
       RTNETLINK answers: No space left on device
       We have an error talking to the kernel
       ...
      
      Fix this in tcf_skbmod_init(), ensuring that tcf_idr_release() is called
      on the error path when the idr has been reserved, but not yet inserted.
      Also, don't test 'ovr' in the error path, to avoid a 'replace' failure
      implicitly become a 'delete' that leaks refcount in act_skbmod module:
      
       # rmmod act_skbmod; modprobe act_skbmod
       # tc action add action skbmod swap mac index 100
       # tc action add action skbmod swap mac continue index 100
       RTNETLINK answers: File exists
       We have an error talking to the kernel
       # tc action replace action skbmod swap mac continue index 100
       RTNETLINK answers: Cannot allocate memory
       We have an error talking to the kernel
       # tc action list action skbmod
       #
       # rmmod  act_skbmod
       rmmod: ERROR: Module act_skbmod is in use
      
      Fixes: 65a206c0 ("net/sched: Change act_api and act_xxx modules to use IDR")
      Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f29cdfbe
    • Davide Caratti's avatar
      net/sched: fix idr leak in the error path of tcf_vlan_init() · d7f20015
      Davide Caratti authored
      tcf_vlan_init() can fail after the idr has been successfully reserved.
      When this happens, every subsequent attempt to configure vlan rules using
      the same idr value will systematically fail with -ENOSPC, unless the first
      attempt was done using the 'replace' keyword.
      
       # tc action add action vlan pop index 100
       RTNETLINK answers: Cannot allocate memory
       We have an error talking to the kernel
       # tc action add action vlan pop index 100
       RTNETLINK answers: No space left on device
       We have an error talking to the kernel
       # tc action add action vlan pop index 100
       RTNETLINK answers: No space left on device
       We have an error talking to the kernel
       ...
      
      Fix this in tcf_vlan_init(), ensuring that tcf_idr_release() is called on
      the error path when the idr has been reserved, but not yet inserted. Also,
      don't test 'ovr' in the error path, to avoid a 'replace' failure implicitly
      become a 'delete' that leaks refcount in act_vlan module:
      
       # rmmod act_vlan; modprobe act_vlan
       # tc action add action vlan push id 5 index 100
       # tc action replace action vlan push id 7 index 100
       RTNETLINK answers: Cannot allocate memory
       We have an error talking to the kernel
       # tc action list action vlan
       #
       # rmmod act_vlan
       rmmod: ERROR: Module act_vlan is in use
      
      Fixes: 4c5b9d96 ("act_vlan: VLAN action rewrite to use RCU lock/unlock and update")
      Fixes: 65a206c0 ("net/sched: Change act_api and act_xxx modules to use IDR")
      Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      d7f20015
    • Davide Caratti's avatar
      net/sched: fix idr leak in the error path of __tcf_ipt_init() · 1e46ef17
      Davide Caratti authored
      __tcf_ipt_init() can fail after the idr has been successfully reserved.
      When this happens, subsequent attempts to configure xt/ipt rules using
      the same idr value systematically fail with -ENOSPC:
      
       # tc action add action xt -j LOG --log-prefix test1 index 100
       tablename: mangle hook: NF_IP_POST_ROUTING
               target:  LOG level warning prefix "test1" index 100
       RTNETLINK answers: Cannot allocate memory
       We have an error talking to the kernel
       Command "(null)" is unknown, try "tc actions help".
       # tc action add action xt -j LOG --log-prefix test1 index 100
       tablename: mangle hook: NF_IP_POST_ROUTING
               target:  LOG level warning prefix "test1" index 100
       RTNETLINK answers: No space left on device
       We have an error talking to the kernel
       Command "(null)" is unknown, try "tc actions help".
       # tc action add action xt -j LOG --log-prefix test1 index 100
       tablename: mangle hook: NF_IP_POST_ROUTING
               target:  LOG level warning prefix "test1" index 100
       RTNETLINK answers: No space left on device
       We have an error talking to the kernel
       ...
      
      Fix this in the error path of __tcf_ipt_init(), calling tcf_idr_release()
      in place of tcf_idr_cleanup(). Since tcf_ipt_release() can now be called
      when tcfi_t is NULL, we also need to protect calls to ipt_destroy_target()
      to avoid NULL pointer dereference.
      
      Fixes: 65a206c0 ("net/sched: Change act_api and act_xxx modules to use IDR")
      Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      1e46ef17
    • Davide Caratti's avatar
      net/sched: fix idr leak in the error path of tcp_pedit_init() · 94fa3f92
      Davide Caratti authored
      tcf_pedit_init() can fail to allocate 'keys' after the idr has been
      successfully reserved. When this happens, subsequent attempts to configure
      a pedit rule using the same idr value systematically fail with -ENOSPC:
      
       # tc action add action pedit munge ip ttl set 63 index 100
       RTNETLINK answers: Cannot allocate memory
       We have an error talking to the kernel
       # tc action add action pedit munge ip ttl set 63 index 100
       RTNETLINK answers: No space left on device
       We have an error talking to the kernel
       # tc action add action pedit munge ip ttl set 63 index 100
       RTNETLINK answers: No space left on device
       We have an error talking to the kernel
       ...
      
      Fix this in the error path of tcf_act_pedit_init(), calling
      tcf_idr_release() in place of tcf_idr_cleanup().
      
      Fixes: 65a206c0 ("net/sched: Change act_api and act_xxx modules to use IDR")
      Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      94fa3f92
    • Davide Caratti's avatar
      net/sched: fix idr leak in the error path of tcf_act_police_init() · 5bf7f818
      Davide Caratti authored
      tcf_act_police_init() can fail after the idr has been successfully
      reserved (e.g., qdisc_get_rtab() may return NULL). When this happens,
      subsequent attempts to configure a police rule using the same idr value
      systematiclly fail with -ENOSPC:
      
       # tc action add action police rate 1000 burst 1000 drop index 100
       RTNETLINK answers: Cannot allocate memory
       We have an error talking to the kernel
       # tc action add action police rate 1000 burst 1000 drop index 100
       RTNETLINK answers: No space left on device
       We have an error talking to the kernel
       # tc action add action police rate 1000 burst 1000 drop index 100
       RTNETLINK answers: No space left on device
       ...
      
      Fix this in the error path of tcf_act_police_init(), calling
      tcf_idr_release() in place of tcf_idr_cleanup().
      
      Fixes: 65a206c0 ("net/sched: Change act_api and act_xxx modules to use IDR")
      Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      5bf7f818
    • Davide Caratti's avatar
      net/sched: fix idr leak in the error path of tcf_simp_init() · 60e10b3a
      Davide Caratti authored
      if the kernel fails to duplicate 'sdata', creation of a new action fails
      with -ENOMEM. However, subsequent attempts to install the same action
      using the same value of 'index' systematically fail with -ENOSPC, and
      that value of 'index' will no more be usable by act_simple, until rmmod /
      insmod of act_simple.ko is done:
      
       # tc actions add action simple sdata hello index 100
       # tc actions list action simple
      
              action order 0: Simple <hello>
               index 100 ref 1 bind 0
       # tc actions flush action simple
       # tc actions add action simple sdata hello index 100
       RTNETLINK answers: Cannot allocate memory
       We have an error talking to the kernel
       # tc actions flush action simple
       # tc actions add action simple sdata hello index 100
       RTNETLINK answers: No space left on device
       We have an error talking to the kernel
       # tc actions add action simple sdata hello index 100
       RTNETLINK answers: No space left on device
       We have an error talking to the kernel
       ...
      
      Fix this in the error path of tcf_simp_init(), calling tcf_idr_release()
      in place of tcf_idr_cleanup().
      
      Fixes: 65a206c0 ("net/sched: Change act_api and act_xxx modules to use IDR")
      Suggested-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
      Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      60e10b3a
    • Davide Caratti's avatar
      net/sched: fix idr leak on the error path of tcf_bpf_init() · bbc09e78
      Davide Caratti authored
      when the following command sequence is entered
      
       # tc action add action bpf bytecode '4,40 0 0 12,31 0 1 2048,6 0 0 262144,6 0 0 0' index 100
       RTNETLINK answers: Invalid argument
       We have an error talking to the kernel
       # tc action add action bpf bytecode '4,40 0 0 12,21 0 1 2048,6 0 0 262144,6 0 0 0' index 100
       RTNETLINK answers: No space left on device
       We have an error talking to the kernel
      
      act_bpf correctly refuses to install the first TC rule, because 31 is not
      a valid instruction. However, it refuses to install the second TC rule,
      even if the BPF code is correct. Furthermore, it's no more possible to
      install any other rule having the same value of 'index' until act_bpf
      module is unloaded/inserted again. After the idr has been reserved, call
      tcf_idr_release() instead of tcf_idr_cleanup(), to fix this issue.
      
      Fixes: 65a206c0 ("net/sched: Change act_api and act_xxx modules to use IDR")
      Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      bbc09e78
    • Colin Ian King's avatar
      qede: fix spelling mistake: "registeration" -> "registration" · 3f2176dd
      Colin Ian King authored
      Trivial fix to spelling mistakes in DP_ERR error message text and
      comments
      Signed-off-by: default avatarColin Ian King <colin.king@canonical.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      3f2176dd
    • Colin Ian King's avatar
      bnx2x: fix spelling mistake: "registeration" -> "registration" · 924613d3
      Colin Ian King authored
      Trivial fix to spelling mistake in BNX2X_ERR error message text
      Signed-off-by: default avatarColin Ian King <colin.king@canonical.com>
      Acked-by: default avatarSudarsana Kalluru <Sudarsana.Kalluru@cavium.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      924613d3
    • David S. Miller's avatar
      Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf · 3d27484e
      David S. Miller authored
      Daniel Borkmann says:
      
      ====================
      pull-request: bpf 2018-03-21
      
      The following pull-request contains BPF updates for your *net* tree.
      
      The main changes are:
      
      1) Follow-up fix to the fault injection framework to prevent jump
         optimization on the kprobe by installing a dummy post-handler,
         from Masami.
      
      2) Drop bpf_perf_prog_read_value helper from tracepoint type programs
         which was mistakenly added there and would otherwise crash due to
         wrong input context, from Yonghong.
      
      3) Fix a crash in BPF fs when compiled with clang. Code appears to
         be fine just that clang tries to overly aggressive optimize in
         non C conform ways, therefore fix the kernel's Makefile to
         generally prevent such issues, from Daniel.
      
      4) Skip unnecessary capability checks in bpf syscall, which is otherwise
         triggering unnecessary security hooks on capability checking and
         causing false alarms on unprivileged processes trying to access
         CAP_SYS_ADMIN restricted infra, from Chenbo.
      
      5) Fix the test_bpf.ko module when CONFIG_BPF_JIT_ALWAYS_ON is set
         with regards to a test case that is really just supposed to fail
         on x8_64 JIT but not others, from Thadeu.
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      3d27484e
    • Daniel Borkmann's avatar
      kbuild: disable clang's default use of -fmerge-all-constants · 87e0d4f0
      Daniel Borkmann authored
      Prasad reported that he has seen crashes in BPF subsystem with netd
      on Android with arm64 in the form of (note, the taint is unrelated):
      
        [ 4134.721483] Unable to handle kernel paging request at virtual address 800000001
        [ 4134.820925] Mem abort info:
        [ 4134.901283]   Exception class = DABT (current EL), IL = 32 bits
        [ 4135.016736]   SET = 0, FnV = 0
        [ 4135.119820]   EA = 0, S1PTW = 0
        [ 4135.201431] Data abort info:
        [ 4135.301388]   ISV = 0, ISS = 0x00000021
        [ 4135.359599]   CM = 0, WnR = 0
        [ 4135.470873] user pgtable: 4k pages, 39-bit VAs, pgd = ffffffe39b946000
        [ 4135.499757] [0000000800000001] *pgd=0000000000000000, *pud=0000000000000000
        [ 4135.660725] Internal error: Oops: 96000021 [#1] PREEMPT SMP
        [ 4135.674610] Modules linked in:
        [ 4135.682883] CPU: 5 PID: 1260 Comm: netd Tainted: G S      W       4.14.19+ #1
        [ 4135.716188] task: ffffffe39f4aa380 task.stack: ffffff801d4e0000
        [ 4135.731599] PC is at bpf_prog_add+0x20/0x68
        [ 4135.741746] LR is at bpf_prog_inc+0x20/0x2c
        [ 4135.751788] pc : [<ffffff94ab7ad584>] lr : [<ffffff94ab7ad638>] pstate: 60400145
        [ 4135.769062] sp : ffffff801d4e3ce0
        [...]
        [ 4136.258315] Process netd (pid: 1260, stack limit = 0xffffff801d4e0000)
        [ 4136.273746] Call trace:
        [...]
        [ 4136.442494] 3ca0: ffffff94ab7ad584 0000000060400145 ffffffe3a01bf8f8 0000000000000006
        [ 4136.460936] 3cc0: 0000008000000000 ffffff94ab844204 ffffff801d4e3cf0 ffffff94ab7ad584
        [ 4136.479241] [<ffffff94ab7ad584>] bpf_prog_add+0x20/0x68
        [ 4136.491767] [<ffffff94ab7ad638>] bpf_prog_inc+0x20/0x2c
        [ 4136.504536] [<ffffff94ab7b5d08>] bpf_obj_get_user+0x204/0x22c
        [ 4136.518746] [<ffffff94ab7ade68>] SyS_bpf+0x5a8/0x1a88
      
      Android's netd was basically pinning the uid cookie BPF map in BPF
      fs (/sys/fs/bpf/traffic_cookie_uid_map) and later on retrieving it
      again resulting in above panic. Issue is that the map was wrongly
      identified as a prog! Above kernel was compiled with clang 4.0,
      and it turns out that clang decided to merge the bpf_prog_iops and
      bpf_map_iops into a single memory location, such that the two i_ops
      could then not be distinguished anymore.
      
      Reason for this miscompilation is that clang has the more aggressive
      -fmerge-all-constants enabled by default. In fact, clang source code
      has a comment about it in lib/AST/ExprConstant.cpp on why it is okay
      to do so:
      
        Pointers with different bases cannot represent the same object.
        (Note that clang defaults to -fmerge-all-constants, which can
        lead to inconsistent results for comparisons involving the address
        of a constant; this generally doesn't matter in practice.)
      
      The issue never appeared with gcc however, since gcc does not enable
      -fmerge-all-constants by default and even *explicitly* states in
      it's option description that using this flag results in non-conforming
      behavior, quote from man gcc:
      
        Languages like C or C++ require each variable, including multiple
        instances of the same variable in recursive calls, to have distinct
        locations, so using this option results in non-conforming behavior.
      
      There are also various clang bug reports open on that matter [1],
      where clang developers acknowledge the non-conforming behavior,
      and refer to disabling it with -fno-merge-all-constants. But even
      if this gets fixed in clang today, there are already users out there
      that triggered this. Thus, fix this issue by explicitly adding
      -fno-merge-all-constants to the kernel's Makefile to generically
      disable this optimization, since potentially other places in the
      kernel could subtly break as well.
      
      Note, there is also a flag called -fmerge-constants (not supported
      by clang), which is more conservative and only applies to strings
      and it's enabled in gcc's -O/-O2/-O3/-Os optimization levels. In
      gcc's code, the two flags -fmerge-{all-,}constants share the same
      variable internally, so when disabling it via -fno-merge-all-constants,
      then we really don't merge any const data (e.g. strings), and text
      size increases with gcc (14,927,214 -> 14,942,646 for vmlinux.o).
      
        $ gcc -fverbose-asm -O2 foo.c -S -o foo.S
          -> foo.S lists -fmerge-constants under options enabled
        $ gcc -fverbose-asm -O2 -fno-merge-all-constants foo.c -S -o foo.S
          -> foo.S doesn't list -fmerge-constants under options enabled
        $ gcc -fverbose-asm -O2 -fno-merge-all-constants -fmerge-constants foo.c -S -o foo.S
          -> foo.S lists -fmerge-constants under options enabled
      
      Thus, as a workaround we need to set both -fno-merge-all-constants
      *and* -fmerge-constants in the Makefile in order for text size to
      stay as is.
      
        [1] https://bugs.llvm.org/show_bug.cgi?id=18538Reported-by: default avatarPrasad Sodagudi <psodagud@codeaurora.org>
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Cc: Linus Torvalds <torvalds@linux-foundation.org>
      Cc: Chenbo Feng <fengc@google.com>
      Cc: Richard Smith <richard-llvm@metafoo.co.uk>
      Cc: Chandler Carruth <chandlerc@gmail.com>
      Cc: linux-kernel@vger.kernel.org
      Tested-by: default avatarPrasad Sodagudi <psodagud@codeaurora.org>
      Acked-by: default avatarAlexei Starovoitov <ast@kernel.org>
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      87e0d4f0
  3. 20 Mar, 2018 12 commits