64 lines
2.5 KiB
Diff
64 lines
2.5 KiB
Diff
|
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
|
||
|
|
|