netdev - RE: [PATCH] netfilter: nf_conntrack: use safer way to lock all buckets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <063D6719AE5E284EB5DD2968C1650D6D1CCBD66E@AcuExch.aculab.com>
Date:	Tue, 5 Jan 2016 11:13:55 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'Sasha Levin' <sasha.levin@...cle.com>,
	"pablo@...filter.org" <pablo@...filter.org>,
	"kaber@...sh.net" <kaber@...sh.net>,
	"kadlec@...ckhole.kfki.hu" <kadlec@...ckhole.kfki.hu>,
	"davem@...emloft.net" <davem@...emloft.net>
CC:	"netfilter-devel@...r.kernel.org" <netfilter-devel@...r.kernel.org>,
	"coreteam@...filter.org" <coreteam@...filter.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] netfilter: nf_conntrack: use safer way to lock all
 buckets

From: Sasha Levin
> Sent: 05 January 2016 02:26
> When we need to lock all buckets in the connection hashtable we'd attempt to
> lock 1024 spinlocks, which is way more preemption levels than supported by
> the kernel. Furthermore, this behavior was hidden by checking if lockdep is
> enabled, and if it was - use only 8 buckets(!).
> 
> Fix this by using a global lock and synchronize all buckets on it when we
> need to lock them all. This is pretty heavyweight, but is only done when we
> need to resize the hashtable, and that doesn't happen often enough (or at all).
...
> +static void nf_conntrack_lock_nested(spinlock_t *lock)
> +{
> +	spin_lock_nested(lock, SINGLE_DEPTH_NESTING);
> +	while (unlikely(nf_conntrack_locks_all)) {
> +		spin_unlock(lock);
> +		spin_lock(&nf_conntrack_locks_all_lock);
> +		spin_unlock(&nf_conntrack_locks_all_lock);
> +		spin_lock_nested(lock, SINGLE_DEPTH_NESTING);
> +	}
> +}
...
> @@ -102,16 +126,19 @@ static void nf_conntrack_all_lock(void)
>  {
>  	int i;
> 
> -	for (i = 0; i < CONNTRACK_LOCKS; i++)
> -		spin_lock_nested(&nf_conntrack_locks[i], i);
> +	spin_lock(&nf_conntrack_locks_all_lock);
> +	nf_conntrack_locks_all = true;
> +
> +	for (i = 0; i < CONNTRACK_LOCKS; i++) {
> +		spin_lock(&nf_conntrack_locks[i]);
> +		spin_unlock(&nf_conntrack_locks[i]);
> +	}
>  }

If spin_lock_nested() does anything like what I think its
name suggests then I suspect that deadlocks.

	David


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html