xserver: Branch 'master' - 5 commits
Keith Packard
keithp at kemper.freedesktop.org
Wed Apr 13 18:44:30 PDT 2011
dix/cursor.c | 5 +----
dix/dispatch.c | 12 +++++++++---
hw/xfree86/common/xf86xv.c | 9 ++++++++-
hw/xfree86/loader/loadmod.c | 32 ++++++++++++--------------------
os/connection.c | 7 +++----
render/render.c | 12 +++++++++---
6 files changed, 42 insertions(+), 35 deletions(-)
New commits:
commit 274dca8f2c6707121d45df8015fe7eddb129dec9
Author: Tiago Vignatti <tiago.vignatti at nokia.com>
Date: Mon Apr 4 22:31:42 2011 +0300
dix: don't free stranger pointers inside AllocARGBCursor
This seems a good convention to follow: if pointers are allocate outside a
given function, then free there as well when a failure occurs.
AllocARGBCursor and its callers were mixing up the freeing of resources and
causing a particular double free inside TileScreenSaver (srcbits and mskbits).
Signed-off-by: Tiago Vignatti <tiago.vignatti at nokia.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira at nokia.com>
diff --git a/dix/cursor.c b/dix/cursor.c
index 72a7609..c191c1e 100644
--- a/dix/cursor.c
+++ b/dix/cursor.c
@@ -241,11 +241,8 @@ AllocARGBCursor(unsigned char *psrcbits, unsigned char *pmaskbits,
*ppCurs = NULL;
pCurs = (CursorPtr)calloc(CURSOR_REC_SIZE + CURSOR_BITS_SIZE, 1);
if (!pCurs)
- {
- free(psrcbits);
- free(pmaskbits);
return BadAlloc;
- }
+
bits = (CursorBitsPtr)((char *)pCurs + CURSOR_REC_SIZE);
dixInitPrivates(pCurs, pCurs + 1, PRIVATE_CURSOR);
dixInitPrivates(bits, bits + 1, PRIVATE_CURSOR_BITS)
diff --git a/dix/dispatch.c b/dix/dispatch.c
index 601b14a..192c8c3 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -2976,11 +2976,17 @@ ProcCreateCursor (ClientPtr client)
&pCursor, client, stuff->cid);
if (rc != Success)
- return rc;
- if (!AddResource(stuff->cid, RT_CURSOR, (pointer)pCursor))
- return BadAlloc;
+ goto bail;
+ if (!AddResource(stuff->cid, RT_CURSOR, (pointer)pCursor)) {
+ rc = BadAlloc;
+ goto bail;
+ }
return Success;
+bail:
+ free(srcbits);
+ free(mskbits);
+ return rc;
}
int
diff --git a/render/render.c b/render/render.c
index c5da6d7..ebb1d63 100644
--- a/render/render.c
+++ b/render/render.c
@@ -1705,11 +1705,17 @@ ProcRenderCreateCursor (ClientPtr client)
GetColor(twocolor[1], 0),
&pCursor, client, stuff->cid);
if (rc != Success)
- return rc;
- if (!AddResource(stuff->cid, RT_CURSOR, (pointer)pCursor))
- return BadAlloc;
+ goto bail;
+ if (!AddResource(stuff->cid, RT_CURSOR, (pointer)pCursor)) {
+ rc = BadAlloc;
+ goto bail;
+ }
return Success;
+bail:
+ free(srcbits);
+ free(mskbits);
+ return rc;
}
static int
commit f603061e9482ad5caf1975ba5395b3294852d072
Author: Tiago Vignatti <tiago.vignatti at nokia.com>
Date: Mon Apr 4 21:40:06 2011 +0300
os: fix use after free in EstablishNewConnections
In the case of failure on AllocNewConnection, new_trans_conn cannot be
dereferenced because it's already freed. Swapping the order of this logic fix
the changes introduced in 04956b80431169e0ae713a3e6ba4cdc157ce3a66.
Signed-off-by: Tiago Vignatti <tiago.vignatti at nokia.com>
CC: Jeremy Huddleston <jeremyhu at freedesktop.org>
Reviewed-by: Jeremy Huddleston <jeremyhu at apple.com>
diff --git a/os/connection.c b/os/connection.c
index 5580fab..0c580ab 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -852,15 +852,14 @@ EstablishNewConnections(ClientPtr clientUnused, pointer closure)
_XSERVTransSetOption(new_trans_conn, TRANS_NONBLOCKING, 1);
+ if(trans_conn->flags & TRANS_NOXAUTH)
+ new_trans_conn->flags = new_trans_conn->flags | TRANS_NOXAUTH;
+
if (!AllocNewConnection (new_trans_conn, newconn, connect_time))
{
ErrorConnMax(new_trans_conn);
_XSERVTransClose(new_trans_conn);
}
-
- if(trans_conn->flags & TRANS_NOXAUTH)
- new_trans_conn->flags = new_trans_conn->flags | TRANS_NOXAUTH;
-
}
#ifndef WIN32
}
commit 82498e3c2cce6f515063ecb4b6ae9303e828da00
Author: Tiago Vignatti <tiago.vignatti at nokia.com>
Date: Mon Apr 4 20:25:32 2011 +0300
xfree86: xv: set pointers to NULL in xf86XVFreeAdaptor
As a good practice and for eventual double frees.
The reason of this patch is due the resilience of xf86XVInitAdaptors, where
for any adaptor failure it's able to keep trying registering the following
ones.
I discussed briefly with Pauli and Ville about a bigger refactoring of such
function, doing it in a way to return instantly when a failure happens; after
all that's how mostly of the other driver functions work. Instead, we just
thought that xf86XVInitAdaptors is wise and cool, and eventually other driver
functions should be even following the main idea of resilience.
Signed-off-by: Tiago Vignatti <tiago.vignatti at nokia.com>
Reviewed-by: Jeremy Huddleston <jeremyhu at apple.com>
diff --git a/hw/xfree86/common/xf86xv.c b/hw/xfree86/common/xf86xv.c
index f87af4c..b46dfef 100644
--- a/hw/xfree86/common/xf86xv.c
+++ b/hw/xfree86/common/xf86xv.c
@@ -313,6 +313,7 @@ xf86XVFreeAdaptor(XvAdaptorPtr pAdaptor)
int i;
free(pAdaptor->name);
+ pAdaptor->name = NULL;
if(pAdaptor->pEncodings) {
XvEncodingPtr pEncode = pAdaptor->pEncodings;
@@ -320,9 +321,11 @@ xf86XVFreeAdaptor(XvAdaptorPtr pAdaptor)
for(i = 0; i < pAdaptor->nEncodings; i++, pEncode++)
free(pEncode->name);
free(pAdaptor->pEncodings);
+ pAdaptor->pEncodings = NULL;
}
free(pAdaptor->pFormats);
+ pAdaptor->pFormats = NULL;
if(pAdaptor->pPorts) {
XvPortPtr pPort = pAdaptor->pPorts;
@@ -341,6 +344,7 @@ xf86XVFreeAdaptor(XvAdaptorPtr pAdaptor)
}
}
free(pAdaptor->pPorts);
+ pAdaptor->pPorts = NULL;
}
if(pAdaptor->pAttributes) {
@@ -354,6 +358,8 @@ xf86XVFreeAdaptor(XvAdaptorPtr pAdaptor)
free(pAdaptor->pImages);
free(pAdaptor->devPriv.ptr);
+ pAdaptor->pImages = NULL;
+ pAdaptor->devPriv.ptr = NULL;
}
static Bool
commit 81414c1c836ae30628606545edbf7392d9b3d009
Author: Tiago Vignatti <tiago.vignatti at nokia.com>
Date: Thu Mar 31 23:44:03 2011 +0300
xfree86: xv: fix double free in xf86XVFreeAdaptor
When xf86XVFreeAdaptor is called more than once in xf86XVInitAdaptors (it may,
but not often), the conditional being changed in this patch will always take
true path and will keep freeing pAdaptor->pAttributes, thus letting the system
error-prone.
This patch fix such problem checking for a pointer instead the number of
attributes. Such pointer will be deallocated when xf86XVFreeAdaptor is called
first and will not let the code re-run in the following calls. This is a bit
similar how the surroundings code is already doing.
Signed-off-by: Tiago Vignatti <tiago.vignatti at nokia.com>
Reviewed-by: Jeremy Huddleston <jeremyhu at apple.com>
diff --git a/hw/xfree86/common/xf86xv.c b/hw/xfree86/common/xf86xv.c
index 53ebe8f..f87af4c 100644
--- a/hw/xfree86/common/xf86xv.c
+++ b/hw/xfree86/common/xf86xv.c
@@ -343,12 +343,13 @@ xf86XVFreeAdaptor(XvAdaptorPtr pAdaptor)
free(pAdaptor->pPorts);
}
- if(pAdaptor->nAttributes) {
+ if(pAdaptor->pAttributes) {
XvAttributePtr pAttribute = pAdaptor->pAttributes;
for(i = 0; i < pAdaptor->nAttributes; i++, pAttribute++)
free(pAttribute->name);
free(pAdaptor->pAttributes);
+ pAdaptor->pAttributes = NULL;
}
free(pAdaptor->pImages);
commit 74476b700f1e499a731ba2ddbba87b12b9b5139b
Author: Tiago Vignatti <tiago.vignatti at nokia.com>
Date: Thu Mar 31 17:46:42 2011 +0300
xfree86: loader: use one exit code only for readability
No functional changes. Spaghetti code for the win! \o/
Signed-off-by: Tiago Vignatti <tiago.vignatti at nokia.com>
Reviewed-by: Nicolas Peninguy <nico at lostgeeks.org>
diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index 46ce68b..9f82099 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -483,19 +483,15 @@ LoaderListDirs(const char **subdirlist, const char **patternlist)
char *fp;
char **listing = NULL;
char **save;
+ char **ret = NULL;
int n = 0;
if (!(pathlist = InitPathList(NULL)))
return NULL;
- if (!(subdirs = InitSubdirs(subdirlist))) {
- FreePathList(pathlist);
- return NULL;
- }
- if (!(patterns = InitPatterns(patternlist))) {
- FreePathList(pathlist);
- FreeSubdirs(subdirs);
- return NULL;
- }
+ if (!(subdirs = InitSubdirs(subdirlist)))
+ goto bail;
+ if (!(patterns = InitPatterns(patternlist)))
+ goto bail;
for (elem = pathlist; *elem; elem++) {
for (s = subdirs; *s; s++) {
@@ -529,20 +525,14 @@ LoaderListDirs(const char **subdirlist, const char **patternlist)
save[n] = NULL;
FreeStringList(save);
}
- FreePathList(pathlist);
- FreeSubdirs(subdirs);
- FreePatterns(patterns);
closedir(d);
- return NULL;
+ goto bail;
}
listing[n] = malloc(len + 1);
if (!listing[n]) {
FreeStringList(listing);
- FreePathList(pathlist);
- FreeSubdirs(subdirs);
- FreePatterns(patterns);
closedir(d);
- return NULL;
+ goto bail;
}
strncpy(listing[n], dp->d_name + match[1].rm_so,
len);
@@ -558,11 +548,13 @@ LoaderListDirs(const char **subdirlist, const char **patternlist)
}
if (listing)
listing[n] = NULL;
+ ret = listing;
- FreePathList(pathlist);
- FreeSubdirs(subdirs);
+bail:
FreePatterns(patterns);
- return listing;
+ FreeSubdirs(subdirs);
+ FreePathList(pathlist);
+ return ret;
}
void
More information about the xorg-commit
mailing list