Re: [PATCH nf] netfilter: ctnetlink: remove unnecessary nf_conntrack_expect_lock protection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Pablo,

2017-04-09 5:16 GMT+08:00 Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>:
> On Sat, Apr 01, 2017 at 10:14:24PM +0800, Liping Zhang wrote:
>> @@ -1960,9 +1955,7 @@ static int ctnetlink_new_conntrack(struct net *net, struct sock *ctnl,
>>       err = -EEXIST;
>>       ct = nf_ct_tuplehash_to_ctrack(h);
>>       if (!(nlh->nlmsg_flags & NLM_F_EXCL)) {
>> -             spin_lock_bh(&nf_conntrack_expect_lock);
>>               err = ctnetlink_change_conntrack(ct, cda);
>> -             spin_unlock_bh(&nf_conntrack_expect_lock);
>
> We used to have a central spinlock here.
>
>         spin_lock_bh(&nf_conntrack_lock);
>
> that was removed time ago, so this go converted to use
> nf_conntrack_expect_lock.

This patch should add:

Fixes: ca7433df3a67 ("netfilter: conntrack: seperate expect locking
from nf_conntrack_lock")

Commit ca7433df3a67 add spin_lock_bh(&nf_conntrack_expect_lock) in
nf_ct_remove_expectations, but we also lock the _expect_lock before calling
ctnetlink_change_conntrack, so dead lock will happen:

 spin_lock_bh(&nf_conntrack_expect_lock):
->err = ctnetlink_change_conntrack(ct, cda)
-->ctnetlink_change_helper
--->if (!strcmp(helpname, "")) nf_ct_remove_expectations()
---->spin_lock_bh(&nf_conntrack_expect_lock); //lock _expect_lock
again, dead lock!

Since ctnetlink_change_conntrack is unrelated to nf_conntrack_expect_lock,
so remove it can fix this issue.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux