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
| ||
|
Message-ID: <alpine.DEB.2.00.1010311206290.16129@davide-lnx1> Date: Sun, 31 Oct 2010 12:07:32 -0700 (PDT) From: Davide Libenzi <davidel@...ilserver.org> To: Eric Dumazet <eric.dumazet@...il.com> cc: Alban Crequy <alban.crequy@...labora.co.uk>, David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org> Subject: Re: [PATCH 1/2] af_unix: fix unix_dgram_poll() behavior for EPOLLOUT event On Sun, 31 Oct 2010, Eric Dumazet wrote: > Le samedi 30 octobre 2010 à 22:47 +0100, Alban Crequy a écrit : > > Le Sat, 30 Oct 2010 15:17:58 +0200, > > Eric Dumazet <eric.dumazet@...il.com> a écrit : > > > > > > Problem is the peer_wait, that epoll doesnt seem to be plugged into. > > > > > > > > Bug is in unix_dgram_poll() > > > > > > > > It calls sock_poll_wait( ... &unix_sk(other)->peer_wait,) only if > > > > socket is 'writable'. Its a clear bug > > > > Yes, it looks weird... > > > > > > Try this patch please ? > > > > I will be away from computer and I will continue to work on this from > > the 20th of November. > > OK, no problem. I tried it and it solves the problem. Here is an > official patch submission. > > David, for your convenience, I cooked a patch serie for the 2 patches > against af_unix unix_dgram_poll(). Looks good to me... Acked-by: Davide Libenzi <davidel@...ilserver.org> > The third patch (af_unix: unix_write_space() use keyed wakeups)) can be > applied as is. > > Thanks ! > > [PATCH 1/2] af_unix: fix unix_dgram_poll() behavior for EPOLLOUT event > > Alban Crequy reported a problem with connected dgram af_unix sockets and > provided a test program. epoll() would miss to send an EPOLLOUT event > when a thread unqueues a packet from the other peer, making its receive > queue not full. > > This is because unix_dgram_poll() fails to call sock_poll_wait(file, > &unix_sk(other)->peer_wait, wait); > if the socket is not writeable at the time epoll_ctl(ADD) is called. > > We must call sock_poll_wait(), regardless of 'writable' status, so that > epoll can be notified later of states changes. > > Misc: avoids testing twice (sk->sk_shutdown & RCV_SHUTDOWN) > > Reported-by: Alban Crequy <alban.crequy@...labora.co.uk> > Cc: Davide Libenzi <davidel@...ilserver.org> > Signed-off-by: Eric Dumazet <eric.dumazet@...il.com> > --- > net/unix/af_unix.c | 24 +++++++++--------------- > 1 file changed, 9 insertions(+), 15 deletions(-) > > diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c > index 0ebc777..7375131 100644 > --- a/net/unix/af_unix.c > +++ b/net/unix/af_unix.c > @@ -2072,13 +2072,12 @@ static unsigned int unix_dgram_poll(struct file *file, struct socket *sock, > if (sk->sk_err || !skb_queue_empty(&sk->sk_error_queue)) > mask |= POLLERR; > if (sk->sk_shutdown & RCV_SHUTDOWN) > - mask |= POLLRDHUP; > + mask |= POLLRDHUP | POLLIN | POLLRDNORM; > if (sk->sk_shutdown == SHUTDOWN_MASK) > mask |= POLLHUP; > > /* readable? */ > - if (!skb_queue_empty(&sk->sk_receive_queue) || > - (sk->sk_shutdown & RCV_SHUTDOWN)) > + if (!skb_queue_empty(&sk->sk_receive_queue)) > mask |= POLLIN | POLLRDNORM; > > /* Connection-based need to check for termination and startup */ > @@ -2090,20 +2089,15 @@ static unsigned int unix_dgram_poll(struct file *file, struct socket *sock, > return mask; > } > > - /* writable? */ > writable = unix_writable(sk); > - if (writable) { > - other = unix_peer_get(sk); > - if (other) { > - if (unix_peer(other) != sk) { > - sock_poll_wait(file, &unix_sk(other)->peer_wait, > - wait); > - if (unix_recvq_full(other)) > - writable = 0; > - } > - > - sock_put(other); > + other = unix_peer_get(sk); > + if (other) { > + if (unix_peer(other) != sk) { > + sock_poll_wait(file, &unix_sk(other)->peer_wait, wait); > + if (unix_recvq_full(other)) > + writable = 0; > } > + sock_put(other); > } > > if (writable) > - Davide
Powered by blists - more mailing lists