Attachment 117200 Details for Bug 91316 – os: make sure the clientsWritable fd_set is initialized before use

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

0001-os-make-sure-the-clientsWritable-fd_set-is-initializ.patch (text/plain), 2.64 KB, created by Martin Peres on 2015-07-17 15:14:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Peres

Created: 2015-07-17 15:14:24 UTC

Size: 2.64 KB

patch

obsolete

>From 7cc7ffd25d5e50b54cb942d07d4cb160f20ff9c5 Mon Sep 17 00:00:00 2001
>From: Martin Peres <martin.peres@linux.intel.com>
>Date: Fri, 17 Jul 2015 17:21:26 +0300
>Subject: [PATCH] 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: Karol Herbst <freedesktop@karolherbst.de>
>Reported-by: Tobias Klausmann <tobias.klausmann@mni.thm.de>
>Signed-off-by: Martin Peres <martin.peres@linux.intel.com>
>Tested-by: Martin Peres <martin.peres@linux.intel.com>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91316
>Cc: Ilia Mirkin  <imirkin@alum.mit.edu>
>Cc: Martin Peres <martin.peres@linux.intel.com>
>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>
>---
> 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
>

Actions: View | Splinter Review

Attachments on bug 91316: 117074 | 117075 | 117194 | 117199 | 117200

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.