Blob Blame History Raw
From 77f8517710a724fa1f29de8ad806692782f962bd Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <fziglio@redhat.com>
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 <fziglio@redhat.com>
---
 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