Commit 50c16716 authored by Martin Peres's avatar Martin Peres Committed by Adam Jackson

os: make sure the clientsWritable fd_set is initialized before use

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: 's avatarKarol Herbst <freedesktop@karolherbst.de>
Reported-by: 's avatarTobias Klausmann <tobias.klausmann@mni.thm.de>
Signed-off-by: 's avatarMartin Peres <martin.peres@linux.intel.com>
Tested-by: 's avatarTobias Klausmann <tobias.klausmann@mni.thm.de>
Tested-by: 's avatarMartin Peres <martin.peres@linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91316
Cc: Ilia Mirkin  <imirkin@alum.mit.edu>
Cc: Olivier Fourdan <ofourdan@redhat.com
Cc: Adam Jackson <ajax@redhat.com>
Cc: Alan Coopersmith <alan.coopersmith@oracle.com
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: 's avatarAlan Coopersmith <alan.coopersmith@oracle.com>
parent bb78c464
......@@ -158,6 +158,7 @@ WaitForSomething(int *pClientsReady)
Bool someReady = FALSE;
FD_ZERO(&clientsReadable);
FD_ZERO(&clientsWritable);
if (nready)
SmartScheduleStopTimer();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment