[ovs-dev] [PATCH] datapath: list: Fix double fetch of pointer in hlist_entry_safe()
Pravin B Shelar
pshelar at nicira.com
Fri Jul 26 20:52:24 UTC 2013
Following patch backports commit f65846a1800ef8c48d (list: Fix double
fetch of pointer in hlist_entry_safe()) from upstream kernel.
Thanks to Jesse for helping to debug this issue.
Original commit msg:
list: Fix double fetch of pointer in hlist_entry_safe()
The current version of hlist_entry_safe() fetches the pointer twice,
once to test for NULL and the other to compute the offset back to the
enclosing structure. This is OK for normal lock-based use because in
that case, the pointer cannot change. However, when the pointer is
protected by RCU (as in "rcu_dereference(p)"), then the pointer can
change at any time. This use case can result in the following sequence
of events:
1. CPU 0 invokes hlist_entry_safe(), fetches the RCU-protected
pointer as sees that it is non-NULL.
2. CPU 1 invokes hlist_del_rcu(), deleting the entry that CPU 0
just fetched a pointer to. Because this is the last entry
in the list, the pointer fetched by CPU 0 is now NULL.
3. CPU 0 refetches the pointer, obtains NULL, and then gets a
NULL-pointer crash.
This commit therefore applies gcc's "({ })" statement expression to
create a temporary variable so that the specified pointer is fetched
only once, avoiding the above sequence of events. Please note that
it is the caller's responsibility to use rcu_dereference() as needed.
This allows RCU-protected uses to work correctly without imposing
any additional overhead on the non-RCU case.
Many thanks to Eric Dumazet for spotting root cause!
Reported-by: CAI Qian <caiqian at redhat.com>
Reported-by: Eric Dumazet <eric.dumazet at gmail.com>
Signed-off-by: Paul E. McKenney <paulmck at linux.vnet.ibm.com>
Tested-by: Li Zefan <lizefan at huawei.com>
Bug #17099
Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
---
datapath/linux/compat/include/linux/list.h | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/datapath/linux/compat/include/linux/list.h b/datapath/linux/compat/include/linux/list.h
index 4446779..18cce8a 100644
--- a/datapath/linux/compat/include/linux/list.h
+++ b/datapath/linux/compat/include/linux/list.h
@@ -5,7 +5,9 @@
#ifndef hlist_entry_safe
#define hlist_entry_safe(ptr, type, member) \
- (ptr) ? hlist_entry(ptr, type, member) : NULL
+ ({ typeof(ptr) ____ptr = (ptr); \
+ ____ptr ? hlist_entry(____ptr, type, member) : NULL; \
+ })
#undef hlist_for_each_entry
#define hlist_for_each_entry(pos, head, member) \
--
1.7.1
More information about the dev
mailing list