1. 02 5月, 2019 14 次提交
    • D
      ext4: fix some error pointer dereferences · 8766cc7d
      Dan Carpenter 提交于
      [ Upstream commit 7159a986b4202343f6cca3bb8079ecace5816fd6 ]
      
      We can't pass error pointers to brelse().
      
      Fixes: fb265c9cb49e ("ext4: add ext4_sb_bread() to disambiguate ENOMEM cases")
      Signed-off-by: NDan Carpenter <dan.carpenter@oracle.com>
      Signed-off-by: NTheodore Ts'o <tytso@mit.edu>
      Reviewed-by: NJan Kara <jack@suse.cz>
      Signed-off-by: NSasha Levin <sashal@kernel.org>
      8766cc7d
    • A
      net: mvpp2: fix validate for PPv2.1 · 816e3e22
      Antoine Tenart 提交于
      [ Upstream commit 8b318f30ab4ef9bbc1241e6f8c1db366dbd347f2 ]
      
      The Phylink validate function is the Marvell PPv2 driver makes a check
      on the GoP id. This is valid an has to be done when using PPv2.2 engines
      but makes no sense when using PPv2.1. The check done when using an RGMII
      interface makes sure the GoP id is not 0, but this breaks PPv2.1. Fixes
      it.
      
      Fixes: 0fb628f0f250 ("net: mvpp2: fix phylink handling of invalid PHY modes")
      Signed-off-by: NAntoine Tenart <antoine.tenart@bootlin.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      Signed-off-by: NSasha Levin <sashal@kernel.org>
      816e3e22
    • T
      net/ibmvnic: Fix RTNL deadlock during device reset · e7ddd6af
      Thomas Falcon 提交于
      [ Upstream commit 986103e7920cabc0b910749e77ae5589d3934d52 ]
      
      Commit a5681e20b541 ("net/ibmnvic: Fix deadlock problem
      in reset") made the change to hold the RTNL lock during
      driver reset but still calls netdev_notify_peers, which
      results in a deadlock. Instead, use call_netdevice_notifiers,
      which is functionally the same except that it does not
      take the RTNL lock again.
      
      Fixes: a5681e20b541 ("net/ibmnvic: Fix deadlock problem in reset")
      Signed-off-by: NThomas Falcon <tlfalcon@linux.ibm.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      Signed-off-by: NSasha Levin <sashal@kernel.org>
      e7ddd6af
    • P
      netfilter: nf_tables: bogus EBUSY in helper removal from transaction · ffc1d85e
      Pablo Neira Ayuso 提交于
      [ Upstream commit 8ffcd32f64633926163cdd07a7d295c500a947d1 ]
      
      Proper use counter updates when activating and deactivating the object,
      otherwise, this hits bogus EBUSY error.
      
      Fixes: cd5125d8f518 ("netfilter: nf_tables: split set destruction in deactivate and destroy phase")
      Reported-by: NLaura Garcia <nevola@gmail.com>
      Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: NSasha Levin <sashal@kernel.org>
      ffc1d85e
    • P
      netfilter: nf_tables: bogus EBUSY when deleting set after flush · e313d5da
      Pablo Neira Ayuso 提交于
      [ Upstream commit 273fe3f1006ea5ebc63d6729e43e8e45e32b256a ]
      
      Set deletion after flush coming in the same batch results in EBUSY. Add
      set use counter to track the number of references to this set from
      rules. We cannot rely on the list of bindings for this since such list
      is still populated from the preparation phase.
      Reported-by: NVáclav Zindulka <vaclav.zindulka@tlapnet.cz>
      Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: NSasha Levin <sashal@kernel.org>
      e313d5da
    • P
      netfilter: nf_tables: fix set double-free in abort path · 25ddad73
      Pablo Neira Ayuso 提交于
      [ Upstream commit 40ba1d9b4d19796afc9b7ece872f5f3e8f5e2c13 ]
      
      The abort path can cause a double-free of an anonymous set.
      Added-and-to-be-aborted rule looks like this:
      
      udp dport { 137, 138 } drop
      
      The to-be-aborted transaction list looks like this:
      
      newset
      newsetelem
      newsetelem
      rule
      
      This gets walked in reverse order, so first pass disables the rule, the
      set elements, then the set.
      
      After synchronize_rcu(), we then destroy those in same order: rule, set
      element, set element, newset.
      
      Problem is that the anonymous set has already been bound to the rule, so
      the rule (lookup expression destructor) already frees the set, when then
      cause use-after-free when trying to delete the elements from this set,
      then try to free the set again when handling the newset expression.
      
      Rule releases the bound set in first place from the abort path, this
      causes the use-after-free on set element removal when undoing the new
      element transactions. To handle this, skip new element transaction if
      set is bound from the abort path.
      
      This is still causes the use-after-free on set element removal.  To
      handle this, remove transaction from the list when the set is already
      bound.
      
      Joint work with Florian Westphal.
      
      Fixes: f6ac85858976 ("netfilter: nf_tables: unbind set in rule from commit path")
      Bugzilla: https://bugzilla.netfilter.org/show_bug.cgi?id=1325Acked-by: NFlorian Westphal <fw@strlen.de>
      Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: NSasha Levin <sashal@kernel.org>
      25ddad73
    • P
      netfilter: nft_compat: use .release_ops and remove list of extension · 8906234c
      Pablo Neira Ayuso 提交于
      [ Upstream commit b8e204006340b7aaf32bd2b9806c692f6e0cb38a ]
      
      Add .release_ops, that is called in case of error at a later stage in
      the expression initialization path, ie. .select_ops() has been already
      set up operations and that needs to be undone. This allows us to unwind
      .select_ops from the error path, ie. release the dynamic operations for
      this extension.
      
      Moreover, allocate one single operation instead of recycling them, this
      comes at the cost of consuming a bit more memory per rule, but it
      simplifies the infrastructure.
      Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: NSasha Levin <sashal@kernel.org>
      8906234c
    • F
      netfilter: nft_compat: don't use refcount_inc on newly allocated entry · 4f67e897
      Florian Westphal 提交于
      [ Upstream commit 947e492c0fc2132ae5fca081a9c2952ccaab0404 ]
      
      When I moved the refcount to refcount_t type I missed the fact that
      refcount_inc() will result in use-after-free warning with
      CONFIG_REFCOUNT_FULL=y builds.
      
      The correct fix would be to init the reference count to 1 at allocation
      time, but, unfortunately we cannot do this, as we can't undo that
      in case something else fails later in the batch.
      
      So only solution I see is to special-case the 'new entry' condition
      and replace refcount_inc() with a "delayed" refcount_set(1) in this case,
      as done here.
      
      The .activate callback can be removed to simplify things, we only
      need to make sure that deactivate() decrements/unlinks the entry
      from the list at end of transaction phase (commit or abort).
      
      Fixes: 12c44aba6618 ("netfilter: nft_compat: use refcnt_t type for nft_xt reference count")
      Reported-by: NJordan Glover <Golden_Miller83@protonmail.ch>
      Signed-off-by: NFlorian Westphal <fw@strlen.de>
      Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: NSasha Levin <sashal@kernel.org>
      4f67e897
    • P
      netfilter: nf_tables: unbind set in rule from commit path · af26f3e2
      Pablo Neira Ayuso 提交于
      Anonymous sets that are bound to rules from the same transaction trigger
      a kernel splat from the abort path due to double set list removal and
      double free.
      
      This patch updates the logic to search for the transaction that is
      responsible for creating the set and disable the set list removal and
      release, given the rule is now responsible for this. Lookup is reverse
      since the transaction that adds the set is likely to be at the tail of
      the list.
      
      Moreover, this patch adds the unbind step to deliver the event from the
      commit path.  This should not be done from the worker thread, since we
      have no guarantees of in-order delivery to the listener.
      
      This patch removes the assumption that both activate and deactivate
      callbacks need to be provided.
      
      Fixes: cd5125d8f518 ("netfilter: nf_tables: split set destruction in deactivate and destroy phase")
      Reported-by: NMikhail Morfikov <mmorfikov@gmail.com>
      Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
      af26f3e2
    • F
      netfilter: nf_tables: warn when expr implements only one of activate/deactivate · 27458b54
      Florian Westphal 提交于
      ->destroy is only allowed to free data, or do other cleanups that do not
      have side effects on other state, such as visibility to other netlink
      requests.
      
      Such things need to be done in ->deactivate.
      As a transaction can fail, we need to make sure we can undo such
      operations, therefore ->activate() has to be provided too.
      
      So print a warning and refuse registration if expr->ops provides
      only one of the two operations.
      
      v2: fix nft_expr_check_ops to not repeat same check twice (Jones Desougi)
      Signed-off-by: NFlorian Westphal <fw@strlen.de>
      Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
      27458b54
    • F
      netfilter: nft_compat: destroy function must not have side effects · cb2e343d
      Florian Westphal 提交于
      The nft_compat destroy function deletes the nft_xt object from a list.
      This isn't allowed anymore. Destroy functions are called asynchronously,
      i.e. next batch can find the object that has a pending ->destroy()
      invocation:
      
      cpu0                       cpu1
       worker
         ->destroy               for_each_entry()
      	                     if (x == ...
      			        return x->ops;
           list_del(x)
           kfree_rcu(x)
                                 expr->ops->... // ops was free'd
      
      To resolve this, the list_del needs to occur before the transaction
      mutex gets released.  nf_tables has a 'deactivate' hook for this
      purpose, so use that to unlink the object from the list.
      
      Fixes: 0935d5588400 ("netfilter: nf_tables: asynchronous release")
      Reported-by: NTaehee Yoo <ap420073@gmail.com>
      Signed-off-by: NFlorian Westphal <fw@strlen.de>
      Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
      cb2e343d
    • F
      netfilter: nf_tables: split set destruction in deactivate and destroy phase · 3dbba8eb
      Florian Westphal 提交于
      [ Upstream commit cd5125d8f51882279f50506bb9c7e5e89dc9bef3 ]
      
      Splits unbind_set into destroy_set and unbinding operation.
      
      Unbinding removes set from lists (so new transaction would not
      find it anymore) but keeps memory allocated (so packet path continues
      to work).
      
      Rebind function is added to allow unrolling in case transaction
      that wants to remove set is aborted.
      
      Destroy function is added to free the memory, but this could occur
      outside of transaction in the future.
      Signed-off-by: NFlorian Westphal <fw@strlen.de>
      Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: NSasha Levin <sashal@kernel.org>
      3dbba8eb
    • F
      netfilter: nft_compat: make lists per netns · 7693bae6
      Florian Westphal 提交于
      [ Upstream commit cf52572ebbd7189a1966c2b5fc34b97078cd1dce ]
      
      There are two problems with nft_compat since the netlink config
      plane uses a per-netns mutex:
      
      1. Concurrent add/del accesses to the same list
      2. accesses to a list element after it has been free'd already.
      
      This patch fixes the first problem.
      
      Freeing occurs from a work queue, after transaction mutexes have been
      released, i.e., it still possible for a new transaction (even from
      same net ns) to find the to-be-deleted expression in the list.
      
      The ->destroy functions are not allowed to have any such side effects,
      i.e. the list_del() in the destroy function is not allowed.
      
      This part of the problem is solved in the next patch.
      I tried to make this work by serializing list access via mutex
      and by moving list_del() to a deactivate callback, but
      Taehee spotted following race on this approach:
      
        NET #0                          NET #1
         >select_ops()
         ->init()
                                         ->select_ops()
         ->deactivate()
         ->destroy()
            nft_xt_put()
             kfree_rcu(xt, rcu_head);
                                         ->init() <-- use-after-free occurred.
      
      Unfortunately, we can't increment reference count in
      select_ops(), because we can't undo the refcount increase in
      case a different expression fails in the same batch.
      
      (The destroy hook will only be called in case the expression
       was initialized successfully).
      
      Fixes: f102d66b ("netfilter: nf_tables: use dedicated mutex to guard transactions")
      Reported-by: NTaehee Yoo <ap420073@gmail.com>
      Signed-off-by: NFlorian Westphal <fw@strlen.de>
      Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: NSasha Levin <sashal@kernel.org>
      7693bae6
    • F
      netfilter: nft_compat: use refcnt_t type for nft_xt reference count · db99f122
      Florian Westphal 提交于
      [ Upstream commit 12c44aba6618b7f6c437076e5722237190f6cd5f ]
      
      Using standard integer type was fine while all operations on it were
      guarded by the nftnl subsys mutex.
      
      This isn't true anymore:
      1. transactions are guarded only by a pernet mutex, so concurrent
         rule manipulation in different netns is racy
      2. the ->destroy hook runs from a work queue after the transaction
         mutex has been released already.
      
      cpu0                           cpu1 (net 1)        cpu2 (net 2)
       kworker
          nft_compat->destroy        nft_compat->init    nft_compat->init
            if (--nft_xt->ref == 0)   nft_xt->ref++        nft_xt->ref++
      
      Switch to refcount_t.  Doing this however only fixes a minor aspect,
      nft_compat also performs linked-list operations in an unsafe way.
      
      This is addressed in the next two patches.
      
      Fixes: f102d66b ("netfilter: nf_tables: use dedicated mutex to guard transactions")
      Fixes: 0935d5588400 ("netfilter: nf_tables: asynchronous release")
      Reported-by: NTaehee Yoo <ap420073@gmail.com>
      Signed-off-by: NFlorian Westphal <fw@strlen.de>
      Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: NSasha Levin <sashal@kernel.org>
      db99f122
  2. 27 4月, 2019 26 次提交