diff options
author | Paul E. McKenney <paulmck@linux.vnet.ibm.com> | 2013-03-09 10:38:41 -0500 |
---|---|---|
committer | Paul E. McKenney <paulmck@linux.vnet.ibm.com> | 2013-03-14 16:18:30 -0400 |
commit | f65846a1800ef8c48d1ae1973c30dae4c356a800 (patch) | |
tree | 87bd07485309f57d4002a5cd634636e9a2ce3025 /include | |
parent | f6161aa153581da4a3867a2d1a7caf4be19b6ec9 (diff) |
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@redhat.com>
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Tested-by: Li Zefan <lizefan@huawei.com>
Diffstat (limited to 'include')
-rw-r--r-- | include/linux/list.h | 4 |
1 files changed, 3 insertions, 1 deletions
diff --git a/include/linux/list.h b/include/linux/list.h index d991cc147c98..6a1f8df9144b 100644 --- a/include/linux/list.h +++ b/include/linux/list.h | |||
@@ -667,7 +667,9 @@ static inline void hlist_move_list(struct hlist_head *old, | |||
667 | pos = n) | 667 | pos = n) |
668 | 668 | ||
669 | #define hlist_entry_safe(ptr, type, member) \ | 669 | #define hlist_entry_safe(ptr, type, member) \ |
670 | (ptr) ? hlist_entry(ptr, type, member) : NULL | 670 | ({ typeof(ptr) ____ptr = (ptr); \ |
671 | ____ptr ? hlist_entry(____ptr, type, member) : NULL; \ | ||
672 | }) | ||
671 | 673 | ||
672 | /** | 674 | /** |
673 | * hlist_for_each_entry - iterate over list of given type | 675 | * hlist_for_each_entry - iterate over list of given type |