[ovs-dev] [PATCH] datapath: list: Fix double fetch of pointer in hlist_entry_safe()
Pravin Shelar
pshelar at nicira.com
Sat Jul 27 00:45:02 UTC 2013
On Fri, Jul 26, 2013 at 1:57 PM, Jesse Gross <jesse at nicira.com> wrote:
> On Fri, Jul 26, 2013 at 1:52 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
>> 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>
>
> Acked-by: Jesse Gross <jesse at nicira.com>
>
> Can you also add the OVS-specific symptoms to the commit message
> before you push?
>
I pushed it with updated commit msg.
Thanks.
More information about the dev
mailing list