netdev - Re: [PATCH][NETFILTER]: Fix race between clusterip_config_find_get and _entry

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <48019253.2090007@trash.net>
Date:	Sun, 13 Apr 2008 06:55:47 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	Pavel Emelyanov <xemul@...nvz.org>
CC:	Linux Netdev List <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	Netfilter Development Mailinglist 
	<netfilter-devel@...r.kernel.org>
Subject: Re: [PATCH][NETFILTER]: Fix race between clusterip_config_find_get
 and _entry_put.

[Please remeber to CC netfilter-devel]

Pavel Emelyanov wrote:
> Consider we are putting a clusterip_config entry with the "entries"
> count == 1, and on the other CPU there's a clusterip_config_find_get
> in progress:
> 
> CPU1:							CPU2:
> clusterip_config_entry_put:				clusterip_config_find_get:
> if (atomic_dec_and_test(&c->entries)) { 
> 	/* true */
> 							read_lock_bh(&clusterip_lock);
> 							c = __clusterip_config_find(clusterip);
> 							/* found - it's still in list */
> 							...
> 							atomic_inc(&c->entries);
> 							read_unlock_bh(&clusterip_lock);
> 
> 	write_lock_bh(&clusterip_lock);
> 	list_del(&c->list);
> 	write_unlock_bh(&clusterip_lock);
> 	...
> 	dev_put(c->dev);
> 
> Oops! We have an entry returned by the clusterip_config_find_get,
> which is a) not in list b) has a stale dev pointer.
> 
> The problems will happen when the CPU2 will release the entry - it
> will remove it from the list for the 2nd time, thus spoiling it, and
> will put a stale dev pointer.
> 
> The fix is to make atomic_dec_and_test under the clusterip_lock.

I guess an alternative would have been to use atomic_inc_not_zero
in clusterip_config_find_get. But this is only called very rarely,
so it doesn't really matter.

Applied, thanks.
--
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