Re: [PATCH] UDPCP Communication Protocol
From: Stefani Seibold <stefani@seibold.net>
Date: 2011-01-01 21:27:45
Also in:
lkml
Am Freitag, den 31.12.2010, 13:00 +0100 schrieb Eric Dumazet:
Le vendredi 31 décembre 2010 à 12:25 +0100, Eric Dumazet a écrit :quoted
Le vendredi 31 décembre 2010 à 10:29 +0100, stefani@seibold.net a écrit :quoted
+ if (!list_empty(&usk->destlist)) { + state->sk = (struct sock *)usk; + state->dest = list_first_entry(&usk->destlist, + struct udpcp_dest, list); + sock_hold(state->sk); + + if (atomic_read(&state->sk->sk_refcnt) != 1) { + spin_unlock_irqrestore(&spinlock, flags); + return state; + } + atomic_dec(&state->sk->sk_refcnt); + } +I am trying to understand what you are doing here. It seems racy to me. Apparently, what you want is to take a reference only if actual sk_refcnt is not zero. I suggest using atomic_inc_notzero(&state->sk->sk_refcnt) to avoid the race in atomic_dec().Before you ask why its racy, this is because UDP sockets are RCU protected, and RCU lookups depend on sk_refcnt being zero or not. Doing an sk_refcnt increment/decrement opens a race window for the concurrent lookups.
I still revamped the whole /proc/net/udpcp thing and hope it is now race free.