Commit fbba4900 by Nicolas Capens Committed by Nicolas Capens

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/19788Reviewed-by: 's avatarCorentin Wallez <cwallez@google.com> Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com> Tested-by: 's avatarNicolas Capens <nicolascapens@google.com>
parent 551478a9
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include <sys/shm.h> #include <sys/shm.h>
#include <string.h> #include <string.h>
#include <assert.h> #include <assert.h>
#include <stdlib.h>
namespace sw namespace sw
{ {
...@@ -46,8 +47,11 @@ namespace sw ...@@ -46,8 +47,11 @@ namespace sw
if(!x_display) if(!x_display)
{ {
x_display = libX11->XOpenDisplay(0); x_display = libX11->XOpenDisplay(0);
assert(x_display);
} }
validateWindow();
int screen = DefaultScreen(x_display); int screen = DefaultScreen(x_display);
x_gc = libX11->XDefaultGC(x_display, screen); x_gc = libX11->XDefaultGC(x_display, screen);
int depth = libX11->XDefaultDepth(x_display, screen); int depth = libX11->XDefaultDepth(x_display, screen);
...@@ -63,7 +67,7 @@ namespace sw ...@@ -63,7 +67,7 @@ namespace sw
x_image = libX11->XShmCreateImage(x_display, visual, depth, ZPixmap, 0, &shminfo, width, height); 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.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; shminfo.readOnly = False;
PreviousXErrorHandler = libX11->XSetErrorHandler(XShmErrorHandler); PreviousXErrorHandler = libX11->XSetErrorHandler(XShmErrorHandler);
...@@ -87,9 +91,16 @@ namespace sw ...@@ -87,9 +91,16 @@ namespace sw
{ {
int bytes_per_line = width * 4; int bytes_per_line = width * 4;
int bytes_per_image = height * bytes_per_line; 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); memset(buffer, 0, bytes_per_image);
x_image = libX11->XCreateImage(x_display, visual, depth, ZPixmap, 0, buffer, width, height, 32, bytes_per_line); 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 @@ namespace sw ...@@ -97,11 +108,7 @@ namespace sw
{ {
if(!mit_shm) if(!mit_shm)
{ {
x_image->data = 0;
XDestroyImage(x_image); XDestroyImage(x_image);
delete[] buffer;
buffer = 0;
} }
else else
{ {
...@@ -111,6 +118,9 @@ namespace sw ...@@ -111,6 +118,9 @@ namespace sw
shmctl(shminfo.shmid, IPC_RMID, 0); shmctl(shminfo.shmid, IPC_RMID, 0);
} }
// Last chance to check the window before we close the display.
validateWindow();
if(ownX11) if(ownX11)
{ {
libX11->XCloseDisplay(x_display); libX11->XCloseDisplay(x_display);
...@@ -119,8 +129,11 @@ namespace sw ...@@ -119,8 +129,11 @@ namespace sw
void *FrameBufferX11::lock() void *FrameBufferX11::lock()
{ {
stride = x_image->bytes_per_line; if(x_image)
framebuffer = buffer; {
stride = x_image->bytes_per_line;
framebuffer = x_image->data;
}
return framebuffer; return framebuffer;
} }
...@@ -134,6 +147,8 @@ namespace sw ...@@ -134,6 +147,8 @@ namespace sw
{ {
copy(source); copy(source);
assert(validateWindow());
if(!mit_shm) if(!mit_shm)
{ {
libX11->XPutImage(x_display, x_window, x_gc, x_image, 0, 0, 0, 0, width, height); libX11->XPutImage(x_display, x_window, x_gc, x_image, 0, 0, 0, 0, width, height);
...@@ -175,6 +190,21 @@ namespace sw ...@@ -175,6 +190,21 @@ namespace sw
libX11->XDrawString(x_display, x_window, x_gc, 50, 50, string, strlen(string)); 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) NO_SANITIZE_FUNCTION sw::FrameBuffer *createFrameBuffer(void *display, Window window, int width, int height)
......
...@@ -38,6 +38,8 @@ namespace sw ...@@ -38,6 +38,8 @@ namespace sw
void unlock() override; void unlock() override;
private: private:
bool validateWindow();
bool ownX11; bool ownX11;
Display *x_display; Display *x_display;
Window x_window; Window x_window;
...@@ -47,8 +49,6 @@ namespace sw ...@@ -47,8 +49,6 @@ namespace sw
bool mit_shm; bool mit_shm;
XShmSegmentInfo shminfo; XShmSegmentInfo shminfo;
char *buffer;
}; };
} }
......
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