xserver: Branch 'master' - 4 commits
Keith Packard
keithp at kemper.freedesktop.org
Fri Oct 18 17:18:37 PDT 2013
dix/dixfonts.c | 5 +++++
exa/exa_migration_mixed.c | 11 +++++++----
exa/exa_mixed.c | 11 +++++++----
hw/dmx/glxProxy/glxsingle.c | 30 +++++++++++++++++-------------
hw/dmx/glxProxy/glxvendor.c | 30 +++++++++++++++++-------------
hw/xfree86/dixmods/Makefile.am | 2 +-
6 files changed, 54 insertions(+), 35 deletions(-)
New commits:
commit 73b2660d7273d175d279d22f8ca0c3932a14ff1c
Author: Alan Coopersmith <alan.coopersmith at oracle.com>
Date: Mon Sep 16 21:47:16 2013 -0700
Avoid use-after-free in dix/dixfonts.c: doImageText() [CVE-2013-4396]
Save a pointer to the passed in closure structure before copying it
and overwriting the *c pointer to point to our copy instead of the
original. If we hit an error, once we free(c), reset c to point to
the original structure before jumping to the cleanup code that
references *c.
Since one of the errors being checked for is whether the server was
able to malloc(c->nChars * itemSize), the client can potentially pass
a number of characters chosen to cause the malloc to fail and the
error path to be taken, resulting in the read from freed memory.
Since the memory is accessed almost immediately afterwards, and the
X server is mostly single threaded, the odds of the free memory having
invalid contents are low with most malloc implementations when not using
memory debugging features, but some allocators will definitely overwrite
the memory there, leading to a likely crash.
Reported-by: Pedro Ribeiro <pedrib at gmail.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
Reviewed-by: Julien Cristau <jcristau at debian.org>
diff --git a/dix/dixfonts.c b/dix/dixfonts.c
index feb765d..2e34d37 100644
--- a/dix/dixfonts.c
+++ b/dix/dixfonts.c
@@ -1425,6 +1425,7 @@ doImageText(ClientPtr client, ITclosurePtr c)
GC *pGC;
unsigned char *data;
ITclosurePtr new_closure;
+ ITclosurePtr old_closure;
/* We're putting the client to sleep. We need to
save some state. Similar problem to that handled
@@ -1436,12 +1437,14 @@ doImageText(ClientPtr client, ITclosurePtr c)
err = BadAlloc;
goto bail;
}
+ old_closure = c;
*new_closure = *c;
c = new_closure;
data = malloc(c->nChars * itemSize);
if (!data) {
free(c);
+ c = old_closure;
err = BadAlloc;
goto bail;
}
@@ -1452,6 +1455,7 @@ doImageText(ClientPtr client, ITclosurePtr c)
if (!pGC) {
free(c->data);
free(c);
+ c = old_closure;
err = BadAlloc;
goto bail;
}
@@ -1464,6 +1468,7 @@ doImageText(ClientPtr client, ITclosurePtr c)
FreeScratchGC(pGC);
free(c->data);
free(c);
+ c = old_closure;
err = BadAlloc;
goto bail;
}
commit 8afe20d4e34adcfd29bdf43a01d55335ea2c5dba
Author: Alan Coopersmith <alan.coopersmith at oracle.com>
Date: Mon Sep 16 21:45:29 2013 -0700
Update GLX dependencies now that DRI & DRI2 are builtins, not modules
Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
diff --git a/hw/xfree86/dixmods/Makefile.am b/hw/xfree86/dixmods/Makefile.am
index 9933bc8..3c43640 100644
--- a/hw/xfree86/dixmods/Makefile.am
+++ b/hw/xfree86/dixmods/Makefile.am
@@ -32,7 +32,7 @@ libglx_la_LIBADD = $(top_builddir)/glx/libglx.la $(GLX_SYS_LIBS)
if AIGLX_DRI_LOADER
libglx_la_LIBADD += $(top_builddir)/glx/libglxdri.la
if NO_UNDEFINED
-libglx_la_LIBADD += ../dri/libdri.la ../dri2/libdri2.la
+libglx_la_LIBADD += $(LIBDRM_LIBS) $(PIXMAN_LIBS)
endif
endif
libglx_la_SOURCES = glxmodule.c
commit 2704bdb24a2c7bac65b90e05f1a68438b34ecf58
Author: Alan Coopersmith <alan.coopersmith at oracle.com>
Date: Tue Sep 3 22:53:28 2013 -0700
DMX glxproxy: Don't allocate & copy data just to free it unused
Two functions in the DMX glxproxy code loop over all the backend
screens, starting at the highest numbered and counting down to
the lowest.
Previously, for each screen, the code would allocate a buffer
large enough to read the reply from the backend, copy that reply
into the buffer, and then if it wasn't the final screen, free it.
Only the buffer from the final screen is used, to pass on to the
client in the reply.
This modifies it to just immediately discard the responses from
the screens as we loop through it, only doing the allocate & copy
work for the one buffer we pass back to the client.
Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
Reviewed-by: Alex Deucher <aleander.deucher at amd.com>
diff --git a/hw/dmx/glxProxy/glxsingle.c b/hw/dmx/glxProxy/glxsingle.c
index abfb880..679a302 100644
--- a/hw/dmx/glxProxy/glxsingle.c
+++ b/hw/dmx/glxProxy/glxsingle.c
@@ -349,25 +349,29 @@ __glXForwardAllWithReply(__GLXclientState * cl, GLbyte * pc)
* get the reply from the back-end server
*/
_XReply(dpy, (xReply *) &be_reply, 0, False);
- be_buf_size = be_reply.length << 2;
- if (be_buf_size > 0) {
- be_buf = (char *) malloc(be_buf_size);
- if (be_buf) {
- _XRead(dpy, be_buf, be_buf_size);
+ if (s == from_screen) {
+ /* Save data from last reply to send on to client */
+ be_buf_size = be_reply.length << 2;
+ if (be_buf_size > 0) {
+ be_buf = malloc(be_buf_size);
+ if (be_buf) {
+ _XRead(dpy, be_buf, be_buf_size);
+ }
+ else {
+ /* Throw data on the floor */
+ _XEatDataWords(dpy, be_reply.length);
+ return BadAlloc;
+ }
}
- else {
- /* Throw data on the floor */
+ }
+ else {
+ /* Just discard data from all replies before the last one */
+ if (be_reply.length > 0)
_XEatDataWords(dpy, be_reply.length);
- return BadAlloc;
- }
}
UnlockDisplay(dpy);
SyncHandle();
-
- if (s > from_screen && be_buf_size > 0) {
- free(be_buf);
- }
}
/*
diff --git a/hw/dmx/glxProxy/glxvendor.c b/hw/dmx/glxProxy/glxvendor.c
index 50d505c..b475daf 100644
--- a/hw/dmx/glxProxy/glxvendor.c
+++ b/hw/dmx/glxProxy/glxvendor.c
@@ -332,25 +332,29 @@ __glXVForwardAllWithReply(__GLXclientState * cl, GLbyte * pc)
* get the reply from the back-end server
*/
_XReply(dpy, (xReply *) &be_reply, 0, False);
- be_buf_size = be_reply.length << 2;
- if (be_buf_size > 0) {
- be_buf = (char *) malloc(be_buf_size);
- if (be_buf) {
- _XRead(dpy, be_buf, be_buf_size);
+ if (s == from_screen) {
+ /* Save data from last reply to send on to client */
+ be_buf_size = be_reply.length << 2;
+ if (be_buf_size > 0) {
+ be_buf = malloc(be_buf_size);
+ if (be_buf) {
+ _XRead(dpy, be_buf, be_buf_size);
+ }
+ else {
+ /* Throw data on the floor */
+ _XEatDataWords(dpy, be_reply.length);
+ return BadAlloc;
+ }
}
- else {
- /* Throw data on the floor */
+ }
+ else {
+ /* Just discard data from all replies before the last one */
+ if (be_reply.length > 0)
_XEatDataWords(dpy, be_reply.length);
- return BadAlloc;
- }
}
UnlockDisplay(dpy);
SyncHandle();
-
- if (s > from_screen && be_buf_size > 0) {
- free(be_buf);
- }
}
/*
commit 6c06c268adbab63ebe7490489aa030724cbdc54b
Author: Alan Coopersmith <alan.coopersmith at oracle.com>
Date: Sun Aug 18 18:02:49 2013 -0700
Skip damage calls if DamageCreate fails in exa functions
Fixes parfait errors such as:
Null pointer dereference (CWE 476): Write to null pointer pDamage
at line 1833 of miext/damage/damage.c in function 'DamageRegister'.
Function DamageCreate may return constant 'NULL' at line 1775,
called at line 232 of exa/exa_migration_mixed.c
in function 'exaPrepareAccessReg_mixed'.
Constant 'NULL' passed into function DamageRegister,
argument pDamage, from call at line 237.
Null pointer introduced at line 1775 of miext/damage/damage.c
in function 'DamageCreate'.
Null pointer dereference (CWE 476): Write to null pointer pDamage
at line 1833 of miext/damage/damage.c in function 'DamageRegister'.
Function DamageCreate may return constant 'NULL' at line 1775,
called at line 104 of exa/exa_mixed.c
in function 'exaCreatePixmap_mixed'.
Constant 'NULL' passed into function DamageRegister,
argument pDamage, from call at line 109.
Null pointer introduced at line 1775 of miext/damage/damage.c
in function 'DamageCreate'.
Checks are similar to handling results of other calls to DamageCreate.
[ This bug was found by the Parfait 1.3.0 bug checking tool.
http://labs.oracle.com/pls/apex/f?p=labs:49:::::P49_PROJECT_ID:13 ]
Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
diff --git a/exa/exa_migration_mixed.c b/exa/exa_migration_mixed.c
index 5e0bf15..cf66327 100644
--- a/exa/exa_migration_mixed.c
+++ b/exa/exa_migration_mixed.c
@@ -233,10 +233,13 @@ exaPrepareAccessReg_mixed(PixmapPtr pPixmap, int index, RegionPtr pReg)
pPixmap->drawable.pScreen,
pPixmap);
- DamageRegister(&pPixmap->drawable, pExaPixmap->pDamage);
- /* This ensures that pending damage reflects the current operation. */
- /* This is used by exa to optimize migration. */
- DamageSetReportAfterOp(pExaPixmap->pDamage, TRUE);
+ if (pExaPixmap->pDamage) {
+ DamageRegister(&pPixmap->drawable, pExaPixmap->pDamage);
+ /* This ensures that pending damage reflects the current
+ * operation. This is used by exa to optimize migration.
+ */
+ DamageSetReportAfterOp(pExaPixmap->pDamage, TRUE);
+ }
if (has_gpu_copy) {
exaPixmapDirty(pPixmap, 0, 0, pPixmap->drawable.width,
diff --git a/exa/exa_mixed.c b/exa/exa_mixed.c
index 3e2dcf2..b43dfec 100644
--- a/exa/exa_mixed.c
+++ b/exa/exa_mixed.c
@@ -106,10 +106,13 @@ exaCreatePixmap_mixed(ScreenPtr pScreen, int w, int h, int depth,
pPixmap->drawable.pScreen,
pPixmap);
- DamageRegister(&pPixmap->drawable, pExaPixmap->pDamage);
- /* This ensures that pending damage reflects the current operation. */
- /* This is used by exa to optimize migration. */
- DamageSetReportAfterOp(pExaPixmap->pDamage, TRUE);
+ if (pExaPixmap->pDamage) {
+ DamageRegister(&pPixmap->drawable, pExaPixmap->pDamage);
+ /* This ensures that pending damage reflects the current
+ * operation. This is used by exa to optimize migration.
+ */
+ DamageSetReportAfterOp(pExaPixmap->pDamage, TRUE);
+ }
}
}
More information about the xorg-commit
mailing list