lkml.org 
[lkml]   [2013]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] rcu: fix a race in hlist_nulls_for_each_entry_rcu macro
On 25.05.2013 15:37, Paul E. McKenney wrote:
>> 2) A problem occurs when restart_condition is true and we jump to the begin label.
>> We do not recalculate (head + offsetof(head, first)) address, we just dereference
>> again the OLD (head->first) pointer. So, we get a node, that WAS the first node in a
>> previous time instead of current first node. That node can be dead, or, for instance,
>> can be a head of another chain.
>
> OK, this is what I was referring to when I said that the RCU list macros
> assume that the list header is static (or equivalently, appropriately
> protected).
>
> With some_func() as written above, you would need to return some sort
> of failure indication from some_func(), and have the caller refetch head.
> Otherwise, as far as gcc is concerned, the value of the parameter head
> is constant throughout some_func().

Personally I have nothing against this approach, but I'm not sure, if
networking maintainers will adopt corresponding patch. I'll try to find it out.

>
>> It is correct from gcc's point of view, since it doesn't expect the head pointer
>> to change, and this address is just (head + constant offset).
>
> Agreed.
>
> How does the caller calculate the value to pass in through the argument "head"?

struct sock *__udp4_lib_lookup(struct net *net, __be32 saddr,
__be16 sport, __be32 daddr, __be16 dport,
int dif, struct udp_table *udptable)
{
...
unsigned int hash2, slot2, slot = udp_hashfn(net, hnum, udptable->mask);
struct udp_hslot *hslot2, *hslot = &udptable->hash[slot];
int score, badness;

rcu_read_lock();
if (hslot->count > 10) {
hash2 = udp4_portaddr_hash(net, daddr, hnum);
slot2 = hash2 & udptable->mask;
hslot2 = &udptable->hash2[slot2];
...
result = udp4_lib_lookup2(net, saddr, sport,
daddr, hnum, dif,
hslot2, slot2);
...
}

static struct sock *udp4_lib_lookup2(struct net *net,
__be32 saddr, __be16 sport,
__be32 daddr, unsigned int hnum, int dif,
struct udp_hslot *hslot2, unsigned int slot2)
{
...
udp_portaddr_for_each_entry_rcu(sk, node, &hslot2->head) {
...

Thank you!

Regards,
Roman



\
 
 \ /
  Last update: 2013-05-27 14:21    [W:0.120 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site