lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 30 Jun 2009 12:13:40 -0700 (PDT) From: Davide Libenzi <davidel@...ilserver.org> To: Jiri Olsa <jolsa@...hat.com> cc: netdev@...r.kernel.org, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, fbl@...hat.com, nhorman@...hat.com, davem@...hat.com, htejun@...il.com, jarkao2@...il.com, oleg@...hat.com, eric.dumazet@...il.com Subject: Re: [PATCHv3 1/2] net: adding memory barrier to the poll and receive callbacks On Tue, 30 Jun 2009, Jiri Olsa wrote: > Adding memory barrier after the poll_wait function, paired with > receive callbacks. Adding fuctions sock_poll_wait and sock_has_sleeper > to wrap the memory barrier. > > Without the memory barrier, following race can happen. > The race fires, when following code paths meet, and the tp->rcv_nxt > and __add_wait_queue updates stay in CPU caches. > > > CPU1 CPU2 > > sys_select receive packet > ... ... > __add_wait_queue update tp->rcv_nxt > ... ... > tp->rcv_nxt check sock_def_readable > ... { > schedule ... > if (sk->sk_sleep && waitqueue_active(sk->sk_sleep)) > wake_up_interruptible(sk->sk_sleep) > ... > } > > If there was no cache the code would work ok, since the wait_queue and > rcv_nxt are opposit to each other. > > Meaning that once tp->rcv_nxt is updated by CPU2, the CPU1 either already > passed the tp->rcv_nxt check and sleeps, or will get the new value for > tp->rcv_nxt and will return with new data mask. > In both cases the process (CPU1) is being added to the wait queue, so the > waitqueue_active (CPU2) call cannot miss and will wake up CPU1. > > The bad case is when the __add_wait_queue changes done by CPU1 stay in its > cache, and so does the tp->rcv_nxt update on CPU2 side. The CPU1 will then > endup calling schedule and sleep forever if there are no more data on the > socket. > +static inline int sk_has_sleeper(struct sock *sk) > +{ > + /* > + * We need to be sure we are in sync with the > + * add_wait_queue modifications to the wait queue. > + * > + * This memory barrier is paired in the sock_poll_wait. > + */ > + smp_mb(); > + return sk->sk_sleep && waitqueue_active(sk->sk_sleep); > +} Jiri, since this is a pretty tricky condition, would you mind to have a reduced version of the patch comment added to the source code? Patch comments are not really useful when you're trying to make sense of some code ;) - Davide -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists