Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1250804
| From | Jason Baron <jbaron@akamai.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll() |
| Date | 2015-10-19 17:10 +0200 |
| Message-ID | <qlkev-82E-43@gated-at.bofh.it> (permalink) |
| References | (4 earlier) <qiScG-1MX-3@gated-at.bofh.it> <qjleG-3bd-1@gated-at.bofh.it> <qjylA-5Hm-9@gated-at.bofh.it> <qjGVQ-1zi-3@gated-at.bofh.it> <ql3nk-ga-19@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 10/18/2015 04:58 PM, Rainer Weikusat wrote: [...] > > The idea behind 'the wait queue' (insofar I'm aware of it) is that it > will be used as list of threads who need to be notified when the > associated event occurs. Since you seem to argue that the run-of-the-mill > algorithm is too slow for this particular case, is there anything to > back this up? > Generally the poll() routines only add to a wait queue once at the beginning, and all subsequent calls to poll() simply check the wakeup conditions. So here you are proposing to add/remove to the wait queue on subsequent invocations of poll(). So the initial patch I did, continued in the usual pattern and only added once on registration or connect(). However, I do think that this is a special case since the registration is on a shared wait queue, and thus having a long list of registered waiters is going to affect all waiters. So I am fine with doing the add/removes in the poll() routine and I agree that the patch below is more compact that what I initially posted. A couple of notes on your patch: 1) In unix_peer_wake_relay() function, 'sk_wq' is an __rcu pointer and thus it requires proper dereferencing. Something like: struct unix_sock *u; struct socket_wq *wq; u = container_of(wait, struct unix_sock, wait); rcu_read_lock(); wq = rcu_dereference(u->sk.sk_wq); if (wq_has_sleeper(wq)) wake_up_interruptible_sync_poll(&wq->wait, key); rcu_read_unlock(); 2) For the case of epoll() in edge triggered mode we need to ensure that when we return -EAGAIN from unix_dgram_sendmsg() when unix_recvq_full() is true, we need to add a unix_peer_wake_connect() call to guarantee a wakeup. Otherwise, we are going to potentially hang there. With these changes (or tell me why they are not needed), I'm happy to ack this patch. Thanks, -Jason -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll() Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-10-18 23:10 +0200
Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll() Jason Baron <jbaron@akamai.com> - 2015-10-19 17:10 +0200
Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll() Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-10-21 00:40 +0200
Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll() Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-10-21 19:40 +0200
[RFC] unix: fix use-after-free in unix_dgram_poll() Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-10-28 17:50 +0100
Re: [RFC] unix: fix use-after-free in unix_dgram_poll() Jason Baron <jbaron@akamai.com> - 2015-10-28 19:00 +0100
Re: [RFC] unix: fix use-after-free in unix_dgram_poll() Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-10-29 15:30 +0100
Re: [RFC] unix: fix use-after-free in unix_dgram_poll()/ 4.2.5 Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-10-30 22:00 +0100
csiph-web