[1/3] libdrm: fix some potential security issues

Commit Message

[e-mail] April 25, 2014, 2:58 p.m. UTC

From: Tim Gore <[e-mail]>

A static analysis of libdrm source code has identified several
potential bugs. This commit addresses the critical issues in
xf86drm.c, which are all possible null pointer dereferences.
NOTE: I have kept to the indenting style already used in this file,
which is a mixture of spaces and tabs.

Signed-off-by: Tim Gore <[e-mail]>
---
 xf86drm.c | 99 +++++++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 64 insertions(+), 35 deletions(-)

Patch

diff --git a/xf86drm.c b/xf86drm.c
index e94f2cd..77b9337 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -191,12 +191,20 @@  drmHashEntry *drmGetEntry(int fd)
     if (!drmHashTable)
 	drmHashTable = drmHashCreate();
 
+    if (!drmHashTable)
+	    return NULL;
+
     if (drmHashLookup(drmHashTable, key, &value)) {
-	entry           = drmMalloc(sizeof(*entry));
-	entry->fd       = fd;
-	entry->f        = NULL;
-	entry->tagTable = drmHashCreate();
-	drmHashInsert(drmHashTable, key, entry);
+	if ((entry = drmMalloc(sizeof(*entry)))) {
+	    entry->fd       = fd;
+	    entry->f        = NULL;
+	    if ((entry->tagTable = drmHashCreate())) {
+		drmHashInsert(drmHashTable, key, entry);
+	    } else {
+		drmFree(entry);
+		entry = NULL;
+	    }
+	}
     } else {
 	entry = value;
     }
@@ -743,6 +751,9 @@  drmVersionPtr drmGetVersion(int fd)
     drmVersionPtr retval;
     drm_version_t *version = drmMalloc(sizeof(*version));
 
+    if (!version)
+	    return NULL;
+
     version->name_len    = 0;
     version->name        = NULL;
     version->date_len    = 0;
@@ -774,7 +785,8 @@  drmVersionPtr drmGetVersion(int fd)
     if (version->desc_len) version->desc[version->desc_len] = '\0';
 
     retval = drmMalloc(sizeof(*retval));
-    drmCopyVersion(retval, version);
+    if (retval)
+	drmCopyVersion(retval, version);
     drmFreeKernelVersion(version);
     return retval;
 }
@@ -797,6 +809,9 @@  drmVersionPtr drmGetLibVersion(int fd)
 {
     drm_version_t *version = drmMalloc(sizeof(*version));
 
+    if (!version)
+	return NULL;
+
     /* Version history:
      *   NOTE THIS MUST NOT GO ABOVE VERSION 1.X due to drivers needing it
      *   revision 1.0.x = original DRM interface with no drmGetLibVersion
@@ -1112,13 +1127,14 @@  int drmClose(int fd)
     unsigned long key    = drmGetKeyFromFd(fd);
     drmHashEntry  *entry = drmGetEntry(fd);
 
-    drmHashDestroy(entry->tagTable);
-    entry->fd       = 0;
-    entry->f        = NULL;
-    entry->tagTable = NULL;
-
+    if (entry) {
+	drmHashDestroy(entry->tagTable);
+	entry->fd       = 0;
+	entry->f        = NULL;
+	entry->tagTable = NULL;
+	drmFree(entry);
+    }
     drmHashDelete(drmHashTable, key);
-    drmFree(entry);
 
     return close(fd);
 }
@@ -1194,14 +1210,19 @@  drmBufInfoPtr drmGetBufInfo(int fd)
 	    return NULL;
 	}
 
-	retval = drmMalloc(sizeof(*retval));
-	retval->count = info.count;
-	retval->list  = drmMalloc(info.count * sizeof(*retval->list));
-	for (i = 0; i < info.count; i++) {
-	    retval->list[i].count     = info.list[i].count;
-	    retval->list[i].size      = info.list[i].size;
-	    retval->list[i].low_mark  = info.list[i].low_mark;
-	    retval->list[i].high_mark = info.list[i].high_mark;
+	if ((retval = drmMalloc(sizeof(*retval)))) {
+	    retval->count = info.count;
+	    if ((retval->list  = drmMalloc(info.count * sizeof(*retval->list)))) {
+		for (i = 0; i < info.count; i++) {
+		    retval->list[i].count     = info.list[i].count;
+		    retval->list[i].size      = info.list[i].size;
+		    retval->list[i].low_mark  = info.list[i].low_mark;
+		    retval->list[i].high_mark = info.list[i].high_mark;
+		}
+	    } else {
+		drmFree(retval);
+		retval = NULL;
+	    }
 	}
 	drmFree(info.list);
 	return retval;
@@ -1247,14 +1268,19 @@  drmBufMapPtr drmMapBufs(int fd)
 	    return NULL;
 	}
 
-	retval = drmMalloc(sizeof(*retval));
-	retval->count = bufs.count;
-	retval->list  = drmMalloc(bufs.count * sizeof(*retval->list));
-	for (i = 0; i < bufs.count; i++) {
-	    retval->list[i].idx     = bufs.list[i].idx;
-	    retval->list[i].total   = bufs.list[i].total;
-	    retval->list[i].used    = 0;
-	    retval->list[i].address = bufs.list[i].address;
+	if (retval = drmMalloc(sizeof(*retval))) {
+	    retval->count = bufs.count;
+	    if ((retval->list  = drmMalloc(bufs.count * sizeof(*retval->list)))) {
+		for (i = 0; i < bufs.count; i++) {
+		    retval->list[i].idx     = bufs.list[i].idx;
+		    retval->list[i].total   = bufs.list[i].total;
+		    retval->list[i].used    = 0;
+		    retval->list[i].address = bufs.list[i].address;
+		}
+	    } else {
+		drmFree(retval);
+		retval = NULL;
+	    }
 	}
 
 	drmFree(bufs.list);
@@ -2099,10 +2125,11 @@  int drmGetInterruptFromBusID(int fd, int busnum, int devnum, int funcnum)
 int drmAddContextTag(int fd, drm_context_t context, void *tag)
 {
     drmHashEntry  *entry = drmGetEntry(fd);
-
-    if (drmHashInsert(entry->tagTable, context, tag)) {
-	drmHashDelete(entry->tagTable, context);
-	drmHashInsert(entry->tagTable, context, tag);
+    if (entry) {
+	if (drmHashInsert(entry->tagTable, context, tag)) {
+	    drmHashDelete(entry->tagTable, context);
+	    drmHashInsert(entry->tagTable, context, tag);
+	}
     }
     return 0;
 }
@@ -2110,8 +2137,10 @@  int drmAddContextTag(int fd, drm_context_t context, void *tag)
 int drmDelContextTag(int fd, drm_context_t context)
 {
     drmHashEntry  *entry = drmGetEntry(fd);
-
-    return drmHashDelete(entry->tagTable, context);
+    if (entry)
+	return drmHashDelete(entry->tagTable, context);
+    else
+	return 1;  /* not found */
 }
 
 void *drmGetContextTag(int fd, drm_context_t context)
@@ -2119,7 +2148,7 @@  void *drmGetContextTag(int fd, drm_context_t context)
     drmHashEntry  *entry = drmGetEntry(fd);
     void          *value;
 
-    if (drmHashLookup(entry->tagTable, context, &value))
+    if (!entry || (drmHashLookup(entry->tagTable, context, &value)))
 	return NULL;
 
     return value;