Thread (23 messages) 23 messages, 7 authors, 2015-12-03

Re: [PATCH v2 net] ipv6: kill sk_dst_lock

From: Eric Dumazet <edumazet@google.com>
Date: 2015-12-03 16:49:30

Hmm... I got lazy yesterday night I sent the same patch from my
laptop, only changelog was updated.

I should have rebased my patch, because the merge of the np->opt patch
had a small fuzz in dccp_v6_connect() :
a :
if (np->opt != NULL)

became

if (opt)

Thanks !


On Thu, Dec 3, 2015 at 8:32 AM, David Miller [off-list ref] wrote:
From: Eric Dumazet <redacted>
Date: Wed, 02 Dec 2015 21:53:57 -0800
quoted
From: Eric Dumazet <edumazet@google.com>

While testing the np->opt RCU conversion, I found that UDP/IPv6 was
using a mixture of xchg() and sk_dst_lock to protect concurrent changes
to sk->sk_dst_cache, leading to possible corruptions and crashes.

ip6_sk_dst_lookup_flow() uses sk_dst_check() anyway, so the simplest
way to fix the mess is to remove sk_dst_lock completely, as we did for
IPv4.

__ip6_dst_store() and ip6_dst_store() share same implementation.

sk_setup_caps() being called with socket lock being held or not,
we have to use sk_dst_set() instead of __sk_dst_set()

Note that I had to move the "np->dst_cookie = rt6_get_cookie(rt);"
in ip6_dst_store() before the sk_setup_caps(sk, dst) call.

This is because ip6_dst_store() can be called from process context,
without any lock held.

As soon as the dst is installed in sk->sk_dst_cache, dst can be freed
from another cpu doing a concurrent ip6_dst_store()

Doing the dst dereference before doing the install is needed to make
sure no use after free would trigger.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
---
v2: added the explanation about rt6_get_cookie(rt) called
before sk_setup_caps()
Applied to 'net', with some fuzz... did you happen to generate this
against net-next by chance?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help