1. 02 May, 2019 16 commits
    • Dongli Zhang's avatar
      loop: do not print warn message if partition scan is successful · 1832b151
      Dongli Zhang authored
      [ Upstream commit 40853d6f ]
      
      Do not print warn message when the partition scan returns 0.
      
      Fixes: d57f3374 ("loop: Move special partition reread handling in loop_clr_fd()")
      Signed-off-by: default avatarDongli Zhang <dongli.zhang@oracle.com>
      Reviewed-by: default avatarJan Kara <jack@suse.cz>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      1832b151
    • Xin Long's avatar
      tipc: handle the err returned from cmd header function · 070e34b6
      Xin Long authored
      [ Upstream commit 2ac695d1 ]
      
      Syzbot found a crash:
      
        BUG: KMSAN: uninit-value in tipc_nl_compat_name_table_dump+0x54f/0xcd0 net/tipc/netlink_compat.c:872
        Call Trace:
          tipc_nl_compat_name_table_dump+0x54f/0xcd0 net/tipc/netlink_compat.c:872
          __tipc_nl_compat_dumpit+0x59e/0xda0 net/tipc/netlink_compat.c:215
          tipc_nl_compat_dumpit+0x63a/0x820 net/tipc/netlink_compat.c:280
          tipc_nl_compat_handle net/tipc/netlink_compat.c:1226 [inline]
          tipc_nl_compat_recv+0x1b5f/0x2750 net/tipc/netlink_compat.c:1265
          genl_family_rcv_msg net/netlink/genetlink.c:601 [inline]
          genl_rcv_msg+0x185f/0x1a60 net/netlink/genetlink.c:626
          netlink_rcv_skb+0x431/0x620 net/netlink/af_netlink.c:2477
          genl_rcv+0x63/0x80 net/netlink/genetlink.c:637
          netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
          netlink_unicast+0xf3e/0x1020 net/netlink/af_netlink.c:1336
          netlink_sendmsg+0x127f/0x1300 net/netlink/af_netlink.c:1917
          sock_sendmsg_nosec net/socket.c:622 [inline]
          sock_sendmsg net/socket.c:632 [inline]
      
        Uninit was created at:
          __alloc_skb+0x309/0xa20 net/core/skbuff.c:208
          alloc_skb include/linux/skbuff.h:1012 [inline]
          netlink_alloc_large_skb net/netlink/af_netlink.c:1182 [inline]
          netlink_sendmsg+0xb82/0x1300 net/netlink/af_netlink.c:1892
          sock_sendmsg_nosec net/socket.c:622 [inline]
          sock_sendmsg net/socket.c:632 [inline]
      
      It was supposed to be fixed on commit 974cb0e3 ("tipc: fix uninit-value
      in tipc_nl_compat_name_table_dump") by checking TLV_GET_DATA_LEN(msg->req)
      in cmd->header()/tipc_nl_compat_name_table_dump_header(), which is called
      ahead of tipc_nl_compat_name_table_dump().
      
      However, tipc_nl_compat_dumpit() doesn't handle the error returned from cmd
      header function. It means even when the check added in that fix fails, it
      won't stop calling tipc_nl_compat_name_table_dump(), and the issue will be
      triggered again.
      
      So this patch is to add the process for the err returned from cmd header
      function in tipc_nl_compat_dumpit().
      
      Reported-by: syzbot+3ce8520484b0d4e260a5@syzkaller.appspotmail.com
      Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      070e34b6
    • Dan Carpenter's avatar
      ext4: fix some error pointer dereferences · 8766cc7d
      Dan Carpenter authored
      [ Upstream commit 7159a986 ]
      
      We can't pass error pointers to brelse().
      
      Fixes: fb265c9c ("ext4: add ext4_sb_bread() to disambiguate ENOMEM cases")
      Signed-off-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
      Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Reviewed-by: default avatarJan Kara <jack@suse.cz>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      8766cc7d
    • Antoine Tenart's avatar
      net: mvpp2: fix validate for PPv2.1 · 816e3e22
      Antoine Tenart authored
      [ Upstream commit 8b318f30 ]
      
      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: 0fb628f0 ("net: mvpp2: fix phylink handling of invalid PHY modes")
      Signed-off-by: default avatarAntoine Tenart <antoine.tenart@bootlin.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      816e3e22
    • Thomas Falcon's avatar
      net/ibmvnic: Fix RTNL deadlock during device reset · e7ddd6af
      Thomas Falcon authored
      [ Upstream commit 986103e7 ]
      
      Commit a5681e20 ("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: a5681e20 ("net/ibmnvic: Fix deadlock problem in reset")
      Signed-off-by: default avatarThomas Falcon <tlfalcon@linux.ibm.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      e7ddd6af
    • Pablo Neira Ayuso's avatar
      netfilter: nf_tables: bogus EBUSY in helper removal from transaction · ffc1d85e
      Pablo Neira Ayuso authored
      [ Upstream commit 8ffcd32f ]
      
      Proper use counter updates when activating and deactivating the object,
      otherwise, this hits bogus EBUSY error.
      
      Fixes: cd5125d8 ("netfilter: nf_tables: split set destruction in deactivate and destroy phase")
      Reported-by: default avatarLaura Garcia <nevola@gmail.com>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      ffc1d85e
    • Pablo Neira Ayuso's avatar
      netfilter: nf_tables: bogus EBUSY when deleting set after flush · e313d5da
      Pablo Neira Ayuso authored
      [ Upstream commit 273fe3f1 ]
      
      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: default avatarVáclav Zindulka <vaclav.zindulka@tlapnet.cz>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      e313d5da
    • Pablo Neira Ayuso's avatar
      netfilter: nf_tables: fix set double-free in abort path · 25ddad73
      Pablo Neira Ayuso authored
      [ Upstream commit 40ba1d9b ]
      
      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: f6ac8585 ("netfilter: nf_tables: unbind set in rule from commit path")
      Bugzilla: https://bugzilla.netfilter.org/show_bug.cgi?id=1325Acked-by: default avatarFlorian Westphal <fw@strlen.de>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      25ddad73
    • Pablo Neira Ayuso's avatar
      netfilter: nft_compat: use .release_ops and remove list of extension · 8906234c
      Pablo Neira Ayuso authored
      [ Upstream commit b8e20400 ]
      
      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: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      8906234c
    • Florian Westphal's avatar
      netfilter: nft_compat: don't use refcount_inc on newly allocated entry · 4f67e897
      Florian Westphal authored
      [ Upstream commit 947e492c ]
      
      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: 12c44aba ("netfilter: nft_compat: use refcnt_t type for nft_xt reference count")
      Reported-by: default avatarJordan Glover <Golden_Miller83@protonmail.ch>
      Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      4f67e897
    • Pablo Neira Ayuso's avatar
      netfilter: nf_tables: unbind set in rule from commit path · af26f3e2
      Pablo Neira Ayuso authored
      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: cd5125d8 ("netfilter: nf_tables: split set destruction in deactivate and destroy phase")
      Reported-by: default avatarMikhail Morfikov <mmorfikov@gmail.com>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      af26f3e2
    • Florian Westphal's avatar
      netfilter: nf_tables: warn when expr implements only one of activate/deactivate · 27458b54
      Florian Westphal authored
      ->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: default avatarFlorian Westphal <fw@strlen.de>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      27458b54
    • Florian Westphal's avatar
      netfilter: nft_compat: destroy function must not have side effects · cb2e343d
      Florian Westphal authored
      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: 0935d558 ("netfilter: nf_tables: asynchronous release")
      Reported-by: default avatarTaehee Yoo <ap420073@gmail.com>
      Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      cb2e343d
    • Florian Westphal's avatar
      netfilter: nf_tables: split set destruction in deactivate and destroy phase · 3dbba8eb
      Florian Westphal authored
      [ Upstream commit cd5125d8 ]
      
      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: default avatarFlorian Westphal <fw@strlen.de>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      3dbba8eb
    • Florian Westphal's avatar
      netfilter: nft_compat: make lists per netns · 7693bae6
      Florian Westphal authored
      [ Upstream commit cf52572e ]
      
      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: default avatarTaehee Yoo <ap420073@gmail.com>
      Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      7693bae6
    • Florian Westphal's avatar
      netfilter: nft_compat: use refcnt_t type for nft_xt reference count · db99f122
      Florian Westphal authored
      [ Upstream commit 12c44aba ]
      
      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: 0935d558 ("netfilter: nf_tables: asynchronous release")
      Reported-by: default avatarTaehee Yoo <ap420073@gmail.com>
      Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      db99f122
  2. 27 Apr, 2019 24 commits