From 77f8517710a724fa1f29de8ad806692782f962bd Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Wed, 29 Jan 2020 09:06:54 +0000 Subject: [PATCH libX11] Fix poll_for_response race condition In poll_for_response is it possible that event replies are skipped and a more up to date message reply is returned. This will cause next poll_for_event call to fail aborting the program. This was proved using some slow ssh tunnel or using some program to slow down server replies (I used a combination of xtrace and strace). How the race happens: - program enters into poll_for_response; - poll_for_event is called but the server didn't still send the reply; - pending_requests is not NULL because we send a request (see call to append_pending_request in _XSend); - xcb_poll_for_reply64 is called from poll_for_response; - xcb_poll_for_reply64 will read from server, at this point server reply with an event (say sequence N) and the reply to our last request (say sequence N+1); - xcb_poll_for_reply64 returns the reply for the request we asked; - last_request_read is set to N+1 sequence in poll_for_response; - poll_for_response returns the response to the request; - poll_for_event is called (for instance from another poll_for_response); - event with sequence N is retrieved; - the N sequence is widen, however, as the "new" number computed from last_request_read is less than N the number is widened to N + 2^32 (assuming last_request_read is still contained in 32 bit); - poll_for_event enters the nested if statement as req is NULL; - we compare the widen N (which now does not fit into 32 bit) with request (which fits into 32 bit) hitting the throw_thread_fail_assert. I propose to change the widen to not go too far from the wide number instead of supposing the result is always bigger than the wide number passed. Signed-off-by: Frediano Ziglio --- src/xcb_io.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/xcb_io.c b/src/xcb_io.c index 6a12d150..2aacbda3 100644 --- a/src/xcb_io.c +++ b/src/xcb_io.c @@ -201,12 +201,10 @@ static int handle_error(Display *dpy, xError *err, Bool in_XReply) } /* Widen a 32-bit sequence number into a 64bit (uint64_t) sequence number. - * Treating the comparison as a 1 and shifting it avoids a conditional branch. */ static void widen(uint64_t *wide, unsigned int narrow) { - uint64_t new = (*wide & ~((uint64_t)0xFFFFFFFFUL)) | narrow; - *wide = new + (((uint64_t)(new < *wide)) << 32); + *wide += (int32_t) (narrow - *wide); } /* Thread-safety rules: -- 2.23.0