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