: Fix race between clusterip_config_find_get and _entry_put.
Subsystems (3)
PATCH 1/1 : Fix race between clusterip_config_find_get and _entry_put.
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.
---
net/ipv4/netfilter/ipt_CLUSTERIP.c +3 -1
@@ -82,8 +82,8 @@ clusterip_config_put(struct clusterip_config *c)static inline void clusterip_config_entry_put (struct clusterip_config *c) { +write_lock_bh (&clusterip_lock);if (atomic_dec_and_test (&c->entries)) { -write_lock_bh (&clusterip_lock);list_del (&c->list);write_unlock_bh (&clusterip_lock);
@@ -96,7 +96,9 @@ clusterip_config_entry_put(struct clusterip_config *c)#ifdef CONFIG_PROC_FS remove_proc_entry (c->pde->name, c->pde->parent);#endif +return ; } +write_unlock_bh (&clusterip_lock); } static struct clusterip_config *
Patrick McHardy
Applied[Please remeber to CC netfilter-devel]
Quoted 21 lines · Pavel Emelyanov
Consider we are putting a
clusterip_configentry with the "entries" count == 1, and on the other CPU there's aclusterip_config_find_getin 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);
Quoted 8 lines
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_testunder theclusterip_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.