Skip to content

Commit

Permalink
list: Fix double fetch of pointer in hlist_entry_safe()
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reported-by: Eric Dumazet <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Tested-by: Li Zefan <[email protected]>
  • Loading branch information
paulmck committed Mar 14, 2013
1 parent f6161aa commit f65846a
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion include/linux/list.h
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,9 @@ static inline void hlist_move_list(struct hlist_head *old,
pos = n)

#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; \
})

/**
* hlist_for_each_entry - iterate over list of given type
Expand Down

0 comments on commit f65846a

Please sign in to comment.