Harden against X11 instability.
Avoid accessing null pointers when an X11 call fails.
Since EGL doesn't own the X11 window, we expect it to be valid
for the duration of the EGL surface. Fail hard if that's not the case.
Bug chromium:833229
Bug chromium:824522
Change-Id: Iba5e3832fe312fb50232a13e2163a022f5048a76
Reviewed-on: https://swiftshader-review.googlesource.com/19788
Reviewed-by: Corentin Wallez <cwallez@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/Main/FrameBufferX11.cpp b/src/Main/FrameBufferX11.cpp
index ca0bb4b..ca56b59 100644
--- a/src/Main/FrameBufferX11.cpp
+++ b/src/Main/FrameBufferX11.cpp
@@ -21,6 +21,7 @@
#include <sys/shm.h>
#include <string.h>
#include <assert.h>
+#include <stdlib.h>
namespace sw
{
@@ -46,8 +47,11 @@
if(!x_display)
{
x_display = libX11->XOpenDisplay(0);
+ assert(x_display);
}
+ validateWindow();
+
int screen = DefaultScreen(x_display);
x_gc = libX11->XDefaultGC(x_display, screen);
int depth = libX11->XDefaultDepth(x_display, screen);
@@ -63,7 +67,7 @@
x_image = libX11->XShmCreateImage(x_display, visual, depth, ZPixmap, 0, &shminfo, width, height);
shminfo.shmid = shmget(IPC_PRIVATE, x_image->bytes_per_line * x_image->height, IPC_CREAT | SHM_R | SHM_W);
- shminfo.shmaddr = x_image->data = buffer = (char*)shmat(shminfo.shmid, 0, 0);
+ shminfo.shmaddr = x_image->data = (char*)shmat(shminfo.shmid, 0, 0);
shminfo.readOnly = False;
PreviousXErrorHandler = libX11->XSetErrorHandler(XShmErrorHandler);
@@ -87,9 +91,16 @@
{
int bytes_per_line = width * 4;
int bytes_per_image = height * bytes_per_line;
- buffer = new char[bytes_per_image];
+ char *buffer = (char*)malloc(bytes_per_image);
memset(buffer, 0, bytes_per_image);
+
x_image = libX11->XCreateImage(x_display, visual, depth, ZPixmap, 0, buffer, width, height, 32, bytes_per_line);
+ assert(x_image);
+
+ if(!x_image)
+ {
+ free(buffer);
+ }
}
}
@@ -97,11 +108,7 @@
{
if(!mit_shm)
{
- x_image->data = 0;
XDestroyImage(x_image);
-
- delete[] buffer;
- buffer = 0;
}
else
{
@@ -111,6 +118,9 @@
shmctl(shminfo.shmid, IPC_RMID, 0);
}
+ // Last chance to check the window before we close the display.
+ validateWindow();
+
if(ownX11)
{
libX11->XCloseDisplay(x_display);
@@ -119,8 +129,11 @@
void *FrameBufferX11::lock()
{
- stride = x_image->bytes_per_line;
- framebuffer = buffer;
+ if(x_image)
+ {
+ stride = x_image->bytes_per_line;
+ framebuffer = x_image->data;
+ }
return framebuffer;
}
@@ -134,6 +147,8 @@
{
copy(source);
+ assert(validateWindow());
+
if(!mit_shm)
{
libX11->XPutImage(x_display, x_window, x_gc, x_image, 0, 0, 0, 0, width, height);
@@ -175,6 +190,21 @@
libX11->XDrawString(x_display, x_window, x_gc, 50, 50, string, strlen(string));
}
}
+
+ bool FrameBufferX11::validateWindow()
+ {
+ // Since we don't own the window, it is the external client code's responsibility
+ // to not destroy it until we're done with it. We help out by validating it.
+ XWindowAttributes windowAttributes;
+ Status status = libX11->XGetWindowAttributes(x_display, x_window, &windowAttributes);
+
+ if(status != True)
+ {
+ abort(); // Fail hard if we can't obtain the window's attributes.
+ }
+
+ return true;
+ }
}
NO_SANITIZE_FUNCTION sw::FrameBuffer *createFrameBuffer(void *display, Window window, int width, int height)
diff --git a/src/Main/FrameBufferX11.hpp b/src/Main/FrameBufferX11.hpp
index f25487b..aa06276 100644
--- a/src/Main/FrameBufferX11.hpp
+++ b/src/Main/FrameBufferX11.hpp
@@ -38,6 +38,8 @@
void unlock() override;
private:
+ bool validateWindow();
+
bool ownX11;
Display *x_display;
Window x_window;
@@ -47,8 +49,6 @@
bool mit_shm;
XShmSegmentInfo shminfo;
-
- char *buffer;
};
}
