Commit 88be4c09 authored by Liping Zhang's avatar Liping Zhang Committed by Pablo Neira Ayuso

netfilter: ctnetlink: fix deadlock due to acquire _expect_lock twice

Currently, ctnetlink_change_conntrack is always protected by _expect_lock,
but this will cause a deadlock when deleting the helper from a conntrack,
as the _expect_lock will be acquired again by nf_ct_remove_expectations:

         CPU0
        ----
  lock(nf_conntrack_expect_lock);
  lock(nf_conntrack_expect_lock);

  *** DEADLOCK ***
  May be due to missing lock nesting notation

  2 locks held by lt-conntrack_gr/12853:
  #0:  (&table[i].mutex){+.+.+.}, at: [<ffffffffa05e2009>]
       nfnetlink_rcv_msg+0x399/0x6a9 [nfnetlink]
  #1:  (nf_conntrack_expect_lock){+.....}, at: [<ffffffffa05f2c1f>]
       ctnetlink_new_conntrack+0x17f/0x408 [nf_conntrack_netlink]

  Call Trace:
   dump_stack+0x85/0xc2
   __lock_acquire+0x1608/0x1680
   ? ctnetlink_parse_tuple_proto+0x10f/0x1c0 [nf_conntrack_netlink]
   lock_acquire+0x100/0x1f0
   ? nf_ct_remove_expectations+0x32/0x90 [nf_conntrack]
   _raw_spin_lock_bh+0x3f/0x50
   ? nf_ct_remove_expectations+0x32/0x90 [nf_conntrack]
   nf_ct_remove_expectations+0x32/0x90 [nf_conntrack]
   ctnetlink_change_helper+0xc6/0x190 [nf_conntrack_netlink]
   ctnetlink_new_conntrack+0x1b2/0x408 [nf_conntrack_netlink]
   nfnetlink_rcv_msg+0x60a/0x6a9 [nfnetlink]
   ? nfnetlink_rcv_msg+0x1b9/0x6a9 [nfnetlink]
   ? nfnetlink_bind+0x1a0/0x1a0 [nfnetlink]
   netlink_rcv_skb+0xa4/0xc0
   nfnetlink_rcv+0x87/0x770 [nfnetlink]

Since the operations are unrelated to nf_ct_expect, so we can drop the
_expect_lock. Also note, after removing the _expect_lock protection,
another CPU may invoke nf_conntrack_helper_unregister, so we should
use rcu_read_lock to protect __nf_conntrack_helper_find invoked by
ctnetlink_change_helper.

Fixes: ca7433df ("netfilter: conntrack: seperate expect locking from nf_conntrack_lock")
Signed-off-by: default avatarLiping Zhang <zlpnobody@gmail.com>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 14e56761
...@@ -1510,23 +1510,29 @@ static int ctnetlink_change_helper(struct nf_conn *ct, ...@@ -1510,23 +1510,29 @@ static int ctnetlink_change_helper(struct nf_conn *ct,
return 0; return 0;
} }
rcu_read_lock();
helper = __nf_conntrack_helper_find(helpname, nf_ct_l3num(ct), helper = __nf_conntrack_helper_find(helpname, nf_ct_l3num(ct),
nf_ct_protonum(ct)); nf_ct_protonum(ct));
if (helper == NULL) if (helper == NULL) {
rcu_read_unlock();
return -EOPNOTSUPP; return -EOPNOTSUPP;
}
if (help) { if (help) {
if (help->helper == helper) { if (help->helper == helper) {
/* update private helper data if allowed. */ /* update private helper data if allowed. */
if (helper->from_nlattr) if (helper->from_nlattr)
helper->from_nlattr(helpinfo, ct); helper->from_nlattr(helpinfo, ct);
return 0; err = 0;
} else } else
return -EBUSY; err = -EBUSY;
} else {
/* we cannot set a helper for an existing conntrack */
err = -EOPNOTSUPP;
} }
/* we cannot set a helper for an existing conntrack */ rcu_read_unlock();
return -EOPNOTSUPP; return err;
} }
static int ctnetlink_change_timeout(struct nf_conn *ct, static int ctnetlink_change_timeout(struct nf_conn *ct,
...@@ -1945,9 +1951,7 @@ static int ctnetlink_new_conntrack(struct net *net, struct sock *ctnl, ...@@ -1945,9 +1951,7 @@ static int ctnetlink_new_conntrack(struct net *net, struct sock *ctnl,
err = -EEXIST; err = -EEXIST;
ct = nf_ct_tuplehash_to_ctrack(h); ct = nf_ct_tuplehash_to_ctrack(h);
if (!(nlh->nlmsg_flags & NLM_F_EXCL)) { if (!(nlh->nlmsg_flags & NLM_F_EXCL)) {
spin_lock_bh(&nf_conntrack_expect_lock);
err = ctnetlink_change_conntrack(ct, cda); err = ctnetlink_change_conntrack(ct, cda);
spin_unlock_bh(&nf_conntrack_expect_lock);
if (err == 0) { if (err == 0) {
nf_conntrack_eventmask_report((1 << IPCT_REPLY) | nf_conntrack_eventmask_report((1 << IPCT_REPLY) |
(1 << IPCT_ASSURED) | (1 << IPCT_ASSURED) |
...@@ -2342,11 +2346,7 @@ ctnetlink_glue_parse(const struct nlattr *attr, struct nf_conn *ct) ...@@ -2342,11 +2346,7 @@ ctnetlink_glue_parse(const struct nlattr *attr, struct nf_conn *ct)
if (ret < 0) if (ret < 0)
return ret; return ret;
spin_lock_bh(&nf_conntrack_expect_lock); return ctnetlink_glue_parse_ct((const struct nlattr **)cda, ct);
ret = ctnetlink_glue_parse_ct((const struct nlattr **)cda, ct);
spin_unlock_bh(&nf_conntrack_expect_lock);
return ret;
} }
static int ctnetlink_glue_exp_parse(const struct nlattr * const *cda, static int ctnetlink_glue_exp_parse(const struct nlattr * const *cda,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment