[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