[Xcb] [PATCH] _xcb_conn_wait: Shut down the connection on unexpected poll() events.

[Aaron Plattner]

From: Jamey Sharp <jamey at minilop.net>

If a client calls close(2) on the connection's file descriptor and then
flushes writes, libxcb causes a hang in the client.

Any flush eventually calls _xcb_out_send() with has the following loop:
   while(ret && *count)
       ret = _xcb_conn_wait(c, &c->out.cond, vector, count);

_xcb_conn_wait(), if built with USE_POLL, gets the POLLNVAL error. It only
checks for POLLIN and POLLOUT though, ignoring the error. Return value is 1,
count is unmodified, leaving us with an endless loop and a client hang.

XTS testcase Xlib3/XConnectionNumber triggers this bug. It creates a display
connection, closes its file descriptor, tries to send a no-op, and then expects
an error.
http://cgit.freedesktop.org/xorg/test/xts/tree/xts5/Xlib3/XConnectionNumber.m

If poll returned POLLHUP or POLLERR, we might see the same result.

If poll returns any event we didn't ask for, this patch causes
_xcb_conn_shutdown() to be invoked and an error returned. This matches the
behaviour if select(2) is used instead of poll(2): select(2) returns -1 and
EBADF for an already closed file descriptor.

I believe this fix both is safe and will handle any similar error. POSIX says
that the only bits poll is permitted to set in revents are those bits that were
set in events, plus POLLHUP, POLLERR, and POLLNVAL. So if we see any flags we
didn't ask for then something has gone wrong.

Patch inspired by earlier proposals from Peter Hutterer and Aaron
Plattner--thanks!

Reported-by: Peter Hutterer <peter.hutterer at who-t.net>
Reported-by: Aaron Plattner <aplattner at nvidia.com>
Signed-off-by: Jamey Sharp <jamey at minilop.net>
Reviewed-by: Aaron Plattner <aplattner at nvidia.com>
Tested-by: Aaron Plattner <aplattner at nvidia.com>
Cc: Peter Hutterer <peter.hutterer at who-t.net>
Cc: Dan Nicholson <dbn.lists at gmail.com>
---
Peter, does this address your concern about the loop not exiting if errno ==
EINTR?  Alternatively, I could set errno = 0.

I left Jamey as the author since this is basically the same as his patch.

 src/xcb_conn.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/src/xcb_conn.c b/src/xcb_conn.c
index 803f7aa..7b0c2ab 100644
--- a/src/xcb_conn.c
+++ b/src/xcb_conn.c
@@ -319,6 +319,13 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
     do {
 #if USE_POLL
         ret = poll(&fd, 1, -1);
+        /* If poll() returns an vent we didn't expect, such as POLLNVAL, treat
+         * it as if it failed. */
+        if(ret >= 0 && (fd.revents & ~fd.events))
+        {
+            ret = -1;
+            break;
+        }
 #else
         ret = select(c->fd + 1, &rfds, &wfds, 0, 0);
 #endif
-- 
1.6.0.4