[PATCH] os: make sure the clientsWritable fd_set is initialized before use

Martin Peres martin.peres at linux.intel.com
Mon Jul 20 00:37:30 PDT 2015


In WaitForSomething(), the fd_set clientsWritable may be used unitialized when
the boolean AnyClientsWriteBlocked is set in the WakeupHandler(). This leads to
a crash in FlushAllOutput() after x11proto's commit
2c94cdb453bc641246cc8b9a876da9799bee1ce7.

The problem did not manifest before because both the XFD_SIZE and the maximum
number of clients were set to 256. As the connectionTranslation table was
initalized for the 256 clients to 0, the test on the index not being 0 was
aborting before dereferencing the client #0.

As of commit 2c94cdb453bc641246cc8b9a876da9799bee1ce7 in x11proto, the XFD_SIZE
got bumped to 512. This lead the OutputPending fd_set to have any fd above 256
to be uninitialized which in turns lead to reading an index after the end of
the ConnectionTranslation table. This index would then be used to find the
client corresponding to the fd marked as pending writes and would also result
to an out-of-bound access which would usually be the fatal one.

Fix this by zeroing the clientsWritable fd_set at the beginning of
WaitForSomething(). In this case, the bottom part of the loop, which would
indirectly call FlushAllOutput, will not do any work but the next call to
select will result in the execution of the right codepath. This is exactly what
we want because we need to know the writable clients before handling them. In
the end, it also makes sure that the fds above MaxClient are initialized,
preventing the crash in FlushAllOutput().

Thanks to everyone involved in tracking this one down!

Reported-by: Karol Herbst <freedesktop at karolherbst.de>
Reported-by: Tobias Klausmann <tobias.klausmann at mni.thm.de>
Signed-off-by: Martin Peres <martin.peres at linux.intel.com>
Tested-by: Tobias Klausmann <tobias.klausmann at mni.thm.de>
Tested-by: Martin Peres <martin.peres at linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91316
Cc: Ilia Mirkin  <imirkin at alum.mit.edu>
Cc: Olivier Fourdan <ofourdan at redhat.com
Cc: Adam Jackson <ajax at redhat.com>
Cc: Alan Coopersmith <alan.coopersmith at oracle.com
Cc: Chris Wilson <chris at chris-wilson.co.uk>
---
 os/WaitFor.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/os/WaitFor.c b/os/WaitFor.c
index 431f1a6..993c14e 100644
--- a/os/WaitFor.c
+++ b/os/WaitFor.c
@@ -158,6 +158,7 @@ WaitForSomething(int *pClientsReady)
     Bool someReady = FALSE;
 
     FD_ZERO(&clientsReadable);
+    FD_ZERO(&clientsWritable);
 
     if (nready)
         SmartScheduleStopTimer();
-- 
2.4.5



More information about the xorg-devel mailing list