[PATCH 00/12] drm: Fix various static analysis issues

[PATCH 00/12] drm: Fix various static analysis issues

[Praveen Paneri]

This patch set fixes various issues reported by a static
analysis tool.

Praveen Paneri (12):
  intel: Validate bo_fake before using.
  intel: Validate output of realloc()
  intel: Use snprintf instead of sprintf
  intel: Validate pointer before using
  xf86drm: Avoid negative array index value
  xf86drmSL: Check function return value
  intel: Validate memory allocations
  drm: Validate memory allocations
  xf86drmSL: Check memory allocation by SL_RANDOM_INIT()
  xf86drmHash: Check memory allocation in HashHash()
  xf86drm: Validate function return value
  xf86drmSL: Add missing function call to SLLocate()

 intel/intel_bufmgr_fake.c | 20 ++++++++----
 intel/intel_bufmgr_gem.c  | 40 ++++++++++++++++++------
 intel/intel_decode.c      | 14 ++++++---
 xf86drm.c                 | 77 ++++++++++++++++++++++++++++++++++++-----------
 xf86drmHash.c             |  9 ++++++
 xf86drmSL.c               |  8 +++++
 6 files changed, 131 insertions(+), 37 deletions(-)

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/12] intel: Validate bo_fake before using.

[Praveen Paneri]

Check on bo_fake before dereferencing the object in functions
evict_lru and evict_mru.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 intel/intel_bufmgr_fake.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/intel/intel_bufmgr_fake.c b/intel/intel_bufmgr_fake.c
index c4828fa..129d344 100644
--- a/intel/intel_bufmgr_fake.c
+++ b/intel/intel_bufmgr_fake.c
@@ -557,8 +557,10 @@ evict_lru(drm_intel_bufmgr_fake *bufmgr_fake, unsigned int max_fence)
 							    max_fence))
 			return 0;
 
-		set_dirty(&bo_fake->bo);
-		bo_fake->block = NULL;
+		if (bo_fake) {
+			set_dirty(&bo_fake->bo);
+			bo_fake->block = NULL;
+		}
 
 		free_block(bufmgr_fake, block, 0);
 		return 1;
@@ -577,11 +579,13 @@ evict_mru(drm_intel_bufmgr_fake *bufmgr_fake)
 	DRMLISTFOREACHSAFEREVERSE(block, tmp, &bufmgr_fake->lru) {
 		drm_intel_bo_fake *bo_fake = (drm_intel_bo_fake *) block->bo;
 
-		if (bo_fake && (bo_fake->flags & BM_NO_FENCE_SUBDATA))
-			continue;
+		if (bo_fake) {
+			if (bo_fake->flags & BM_NO_FENCE_SUBDATA)
+				continue;
 
-		set_dirty(&bo_fake->bo);
-		bo_fake->block = NULL;
+			set_dirty(&bo_fake->bo);
+			bo_fake->block = NULL;
+		}
 
 		free_block(bufmgr_fake, block, 0);
 		return 1;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/12] intel: Validate output of realloc()

[Praveen Paneri]

realloc will return NULL if failed to allocate the extra memory
requested. Return from function if it fails.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 intel/intel_bufmgr_gem.c | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 5a67f53..2f0ced1 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -433,6 +433,8 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo)
 	drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr;
 	drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
 	int index;
+	struct drm_i915_gem_exec_object *exec_objects;
+	drm_intel_bo **exec_bos;
 
 	if (bo_gem->validate_index != -1)
 		return;
@@ -444,12 +446,20 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo)
 		if (new_size == 0)
 			new_size = 5;
 
-		bufmgr_gem->exec_objects =
-		    realloc(bufmgr_gem->exec_objects,
-			    sizeof(*bufmgr_gem->exec_objects) * new_size);
-		bufmgr_gem->exec_bos =
-		    realloc(bufmgr_gem->exec_bos,
+		exec_objects = realloc(bufmgr_gem->exec_objects,
+				sizeof(*bufmgr_gem->exec_objects) * new_size);
+		if (!exec_objects)
+			return;
+
+		bufmgr_gem->exec_objects = exec_objects;
+
+		exec_bos = realloc(bufmgr_gem->exec_bos,
 			    sizeof(*bufmgr_gem->exec_bos) * new_size);
+		if (!exec_bos)
+			return;
+
+		bufmgr_gem->exec_bos = exec_bos;
+
 		bufmgr_gem->exec_size = new_size;
 	}
 
@@ -471,6 +481,8 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence)
 	drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bo->bufmgr;
 	drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *)bo;
 	int index;
+	struct drm_i915_gem_exec_object2 *exec2_objects;
+	drm_intel_bo **exec_bos;
 
 	if (bo_gem->validate_index != -1) {
 		if (need_fence)
@@ -486,12 +498,19 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence)
 		if (new_size == 0)
 			new_size = 5;
 
-		bufmgr_gem->exec2_objects =
-			realloc(bufmgr_gem->exec2_objects,
+		exec2_objects = realloc(bufmgr_gem->exec2_objects,
 				sizeof(*bufmgr_gem->exec2_objects) * new_size);
-		bufmgr_gem->exec_bos =
-			realloc(bufmgr_gem->exec_bos,
+		if (!exec2_objects)
+			return;
+
+		bufmgr_gem->exec2_objects = exec2_objects;
+
+		exec_bos = realloc(bufmgr_gem->exec_bos,
 				sizeof(*bufmgr_gem->exec_bos) * new_size);
+		if (!exec_bos)
+			return;
+
+		bufmgr_gem->exec_bos = exec_bos;
 		bufmgr_gem->exec_size = new_size;
 	}
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 03/12] intel: Use snprintf instead of sprintf

[Praveen Paneri]

We must have upper bound on what we are going to write into a fixed
size buffer.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 intel/intel_decode.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 7d5cbe5..b70d949 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -767,7 +767,7 @@ static void i915_get_instruction_src0(uint32_t *data, int i, char *srcname)
 	char swizzle[100];
 
 	i915_get_instruction_src_name((a0 >> 7) & 0x7, src_nr, srcname);
-	sprintf(swizzle, ".%s%s%s%s", swizzle_x, swizzle_y, swizzle_z,
+	snprintf(swizzle, sizeof(swizzle), ".%s%s%s%s", swizzle_x, swizzle_y, swizzle_z,
 		swizzle_w);
 	if (strcmp(swizzle, ".xyzw") != 0)
 		strcat(srcname, swizzle);
@@ -785,7 +785,7 @@ static void i915_get_instruction_src1(uint32_t *data, int i, char *srcname)
 	char swizzle[100];
 
 	i915_get_instruction_src_name((a1 >> 13) & 0x7, src_nr, srcname);
-	sprintf(swizzle, ".%s%s%s%s", swizzle_x, swizzle_y, swizzle_z,
+	snprintf(swizzle, sizeof(swizzle), ".%s%s%s%s", swizzle_x, swizzle_y, swizzle_z,
 		swizzle_w);
 	if (strcmp(swizzle, ".xyzw") != 0)
 		strcat(srcname, swizzle);
@@ -802,7 +802,7 @@ static void i915_get_instruction_src2(uint32_t *data, int i, char *srcname)
 	char swizzle[100];
 
 	i915_get_instruction_src_name((a2 >> 21) & 0x7, src_nr, srcname);
-	sprintf(swizzle, ".%s%s%s%s", swizzle_x, swizzle_y, swizzle_z,
+	snprintf(swizzle, sizeof(swizzle), ".%s%s%s%s", swizzle_x, swizzle_y, swizzle_z,
 		swizzle_w);
 	if (strcmp(swizzle, ".xyzw") != 0)
 		strcat(srcname, swizzle);
@@ -931,7 +931,7 @@ i915_decode_dcl(struct drm_intel_decode *ctx, int i, char *instr_prefix)
 
 	switch ((d0 >> 19) & 0x3) {
 	case 1:
-		sprintf(dcl_mask, ".%s%s%s%s", dcl_x, dcl_y, dcl_z, dcl_w);
+		snprintf(dcl_mask, sizeof(dcl_mask), ".%s%s%s%s", dcl_x, dcl_y, dcl_z, dcl_w);
 		if (strcmp(dcl_mask, ".") == 0)
 			fprintf(out, "bad (empty) dcl mask\n");
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 04/12] intel: Validate pointer before using

[Praveen Paneri]

Move the dereferencing below the check for valid ctx pointer.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 intel/intel_decode.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index b70d949..5dab9ca 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -3901,12 +3901,14 @@ drm_intel_decode(struct drm_intel_decode *ctx)
 	int ret;
 	unsigned int index = 0;
 	uint32_t devid;
-	int size = ctx->base_count * 4;
+	int size;
 	void *temp;
 
 	if (!ctx)
 		return;
 
+	size = ctx->base_count * 4;
+
 	/* Put a scratch page full of obviously undefined data after
 	 * the batchbuffer.  This lets us avoid a bunch of length
 	 * checking in statically sized packets.
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/12] xf86drm: Avoid negative array index value

[Praveen Paneri]

Variable retcode can be negative as well. Put the correct
condition on it before using it as array index.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 xf86drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xf86drm.c b/xf86drm.c
index e73cddd..1e25424 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -661,7 +661,7 @@ static int drmOpenByName(const char *name, int type)
 	if ((fd = open(proc_name, 0, 0)) >= 0) {
 	    retcode = read(fd, buf, sizeof(buf)-1);
 	    close(fd);
-	    if (retcode) {
+	    if (retcode > 0) {
 		buf[retcode-1] = '\0';
 		for (driver = pt = buf; *pt && *pt != ' '; ++pt)
 		    ;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 06/12] xf86drmSL: Check function return value

[Praveen Paneri]

Validate the return value of SLCreateEntry() before using it.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 xf86drmSL.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xf86drmSL.c b/xf86drmSL.c
index acddb54..45f3906 100644
--- a/xf86drmSL.c
+++ b/xf86drmSL.c
@@ -139,6 +139,7 @@ void *drmSLCreate(void)
     list->magic    = SL_LIST_MAGIC;
     list->level    = 0;
     list->head     = SLCreateEntry(SL_MAX_LEVEL, 0, NULL);
+    if (!list->head) return NULL;
     list->count    = 0;
 
     for (i = 0; i <= SL_MAX_LEVEL; i++) list->head->forward[i] = NULL;
@@ -205,6 +206,7 @@ int drmSLInsert(void *l, unsigned long key, void *value)
     }
 
     entry = SLCreateEntry(level, key, value);
+    if (!entry) return -ENOMEM;
 
 				/* Fix up forward pointers */
     for (i = 0; i <= level; i++) {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 07/12] intel: Validate memory allocations

[Praveen Paneri]

This patch adds check for various malloc/calloc function if they
were able to allocate memory as requested or not. Return
appropriate error if the allocation fails.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 intel/intel_bufmgr_fake.c | 4 ++++
 intel/intel_bufmgr_gem.c  | 3 +++
 intel/intel_decode.c      | 2 ++
 3 files changed, 9 insertions(+)

diff --git a/intel/intel_bufmgr_fake.c b/intel/intel_bufmgr_fake.c
index 129d344..e2b25eb 100644
--- a/intel/intel_bufmgr_fake.c
+++ b/intel/intel_bufmgr_fake.c
@@ -1278,6 +1278,8 @@ drm_intel_fake_emit_reloc(drm_intel_bo *bo, uint32_t offset,
 	if (bo_fake->relocs == NULL) {
 		bo_fake->relocs =
 		    malloc(sizeof(struct fake_buffer_reloc) * MAX_RELOCS);
+		if (!bo_fake->relocs)
+			return -ENOMEM;
 	}
 
 	r = &bo_fake->relocs[bo_fake->nr_relocs++];
@@ -1597,6 +1599,8 @@ drm_intel_bufmgr_fake_init(int fd, unsigned long low_offset,
 	drm_intel_bufmgr_fake *bufmgr_fake;
 
 	bufmgr_fake = calloc(1, sizeof(*bufmgr_fake));
+	if (!bufmgr_fake)
+		return NULL;
 
 	if (pthread_mutex_init(&bufmgr_fake->lock, NULL) != 0) {
 		free(bufmgr_fake);
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 2f0ced1..fd6279e 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -2067,6 +2067,9 @@ aub_write_bo_data(drm_intel_bo *bo, uint32_t offset, uint32_t size)
 	unsigned int i;
 
 	data = malloc(bo->size);
+	if (!data)
+		return;
+
 	drm_intel_bo_get_subdata(bo, offset, size, data);
 
 	/* Easy mode: write out bo with no relocations */
diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 5dab9ca..88267fd 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -3914,6 +3914,8 @@ drm_intel_decode(struct drm_intel_decode *ctx)
 	 * checking in statically sized packets.
 	 */
 	temp = malloc(size + 4096);
+	if (!temp)
+		return;
 	memcpy(temp, ctx->base_data, size);
 	memset((char *)temp + size, 0xd0, 4096);
 	ctx->data = temp;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 08/12] drm: Validate memory allocations

[Praveen Paneri]

This patch adds check on various drmMalloc() calls if they
were able to allocate memory as requested or not. Return
appropriate error if the allocation fails.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 xf86drm.c | 51 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 11 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index 1e25424..373113b 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -186,6 +186,8 @@ drmHashEntry *drmGetEntry(int fd)
 
     if (drmHashLookup(drmHashTable, key, &value)) {
 	entry           = drmMalloc(sizeof(*entry));
+	if (!entry)
+	    return NULL;
 	entry->fd       = fd;
 	entry->f        = NULL;
 	entry->tagTable = drmHashCreate();
@@ -837,6 +839,8 @@ drmVersionPtr drmGetVersion(int fd)
 {
     drmVersionPtr retval;
     drm_version_t *version = drmMalloc(sizeof(*version));
+    if (!version)
+	return NULL;
 
     memclear(*version);
 
@@ -864,7 +868,9 @@ 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;
 }
@@ -886,6 +892,8 @@ drmVersionPtr drmGetVersion(int fd)
 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
@@ -1294,14 +1302,25 @@ drmBufInfoPtr drmGetBufInfo(int fd)
 	}
 
 	retval = drmMalloc(sizeof(*retval));
+	if (!retval) {
+	    drmFree(info.list);
+	    return NULL;
+	}
+
 	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->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;
     }
@@ -1345,13 +1364,23 @@ drmBufMapPtr drmMapBufs(int fd)
 	}
 
 	retval = drmMalloc(sizeof(*retval));
+	if (!retval) {
+	    drmFree(bufs.list);
+            return NULL;
+	}
+
 	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->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);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/12] xf86drmSL: Check memory allocation by SL_RANDOM_INIT()

[Praveen Paneri]

If the allocation fails, return -ENOMEM. Handle the return value
at the caller funtion drmSLInsert() as well.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 xf86drmSL.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xf86drmSL.c b/xf86drmSL.c
index 45f3906..edafe7b 100644
--- a/xf86drmSL.c
+++ b/xf86drmSL.c
@@ -40,6 +40,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <errno.h>
 
 #define SL_MAIN 0
 
@@ -124,6 +125,7 @@ static int SLRandomLevel(void)
     SL_RANDOM_DECL;
 
     SL_RANDOM_INIT(SL_RANDOM_SEED);
+    if (!state) return -ENOMEM;
     
     while ((SL_RANDOM & 0x01) && level < SL_MAX_LEVEL) ++level;
     return level;
@@ -200,6 +202,9 @@ int drmSLInsert(void *l, unsigned long key, void *value)
 
 
     level = SLRandomLevel();
+    if (level < 0)
+	return level;
+
     if (level > list->level) {
 	level = ++list->level;
 	update[level] = list->head;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/12] xf86drmHash: Check memory allocation in HashHash()

[Praveen Paneri]

HASH_RANDOM_INIT() can fail to allocate memory. In such case return an
invalid hash value (0xffffffff) from HashHash() function.
Caller functions check the hash value and act accordingly.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 xf86drmHash.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/xf86drmHash.c b/xf86drmHash.c
index 82cbc2a..56e9af3 100644
--- a/xf86drmHash.c
+++ b/xf86drmHash.c
@@ -70,6 +70,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <errno.h>
 
 #define HASH_MAIN 0
 
@@ -79,6 +80,7 @@
 
 #define HASH_MAGIC 0xdeadbeef
 #define HASH_DEBUG 0
+#define HASH_INVALID 0xffffffff	/* A value that is out of bound */
 #define HASH_SIZE  512		/* Good for about 100 entries */
 				/* If you change this value, you probably
                                    have to change the HashHash hashing
@@ -137,6 +139,8 @@ static unsigned long HashHash(unsigned long key)
     if (!init) {
 	HASH_RANDOM_DECL;
 	HASH_RANDOM_INIT(37);
+	if (!state)
+		return HASH_INVALID;
 	for (i = 0; i < 256; i++) scatter[i] = HASH_RANDOM;
 	HASH_RANDOM_DESTROY;
 	++init;
@@ -203,6 +207,9 @@ static HashBucketPtr HashFind(HashTablePtr table,
 
     if (h) *h = hash;
 
+    if (hash == HASH_INVALID)
+	return NULL;
+
     for (bucket = table->buckets[hash]; bucket; bucket = bucket->next) {
 	if (bucket->key == key) {
 	    if (prev) {
@@ -244,6 +251,7 @@ int drmHashInsert(void *t, unsigned long key, void *value)
     if (table->magic != HASH_MAGIC) return -1; /* Bad magic */
 
     if (HashFind(table, key, &hash)) return 1; /* Already in table */
+    if (hash == HASH_INVALID) return -1;
 
     bucket               = HASH_ALLOC(sizeof(*bucket));
     if (!bucket) return -1;	/* Error */
@@ -267,6 +275,7 @@ int drmHashDelete(void *t, unsigned long key)
 
     bucket = HashFind(table, key, &hash);
 
+    if (hash == HASH_INVALID) return -1;
     if (!bucket) return 1;	/* Not found */
 
     table->buckets[hash] = bucket->next;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 11/12] xf86drm: Validate function return value

[Praveen Paneri]

Return value of drmHashCreate() and drmGetEntry() functions
can be NULL. It should be validated before being used.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 xf86drm.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index 373113b..d3a002a 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -178,20 +178,25 @@ static unsigned long drmGetKeyFromFd(int fd)
 drmHashEntry *drmGetEntry(int fd)
 {
     unsigned long key = drmGetKeyFromFd(fd);
-    void          *value;
+    void          *value = NULL;
     drmHashEntry  *entry;
 
     if (!drmHashTable)
 	drmHashTable = drmHashCreate();
 
-    if (drmHashLookup(drmHashTable, key, &value)) {
+    if (drmHashTable && drmHashLookup(drmHashTable, key, &value)) {
 	entry           = drmMalloc(sizeof(*entry));
 	if (!entry)
 	    return NULL;
 	entry->fd       = fd;
 	entry->f        = NULL;
 	entry->tagTable = drmHashCreate();
-	drmHashInsert(drmHashTable, key, entry);
+	if (entry->tagTable) {
+		drmHashInsert(drmHashTable, key, entry);
+	} else {
+		drmFree(entry);
+		entry = NULL;
+	}
     } else {
 	entry = value;
     }
@@ -1219,6 +1224,8 @@ int drmClose(int fd)
 {
     unsigned long key    = drmGetKeyFromFd(fd);
     drmHashEntry  *entry = drmGetEntry(fd);
+    if(!entry)
+	return -ENOMEM;
 
     drmHashDestroy(entry->tagTable);
     entry->fd       = 0;
@@ -2258,6 +2265,8 @@ 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 (!entry)
+        return -ENOMEM;
 
     if (drmHashInsert(entry->tagTable, context, tag)) {
 	drmHashDelete(entry->tagTable, context);
@@ -2270,13 +2279,18 @@ int drmDelContextTag(int fd, drm_context_t context)
 {
     drmHashEntry  *entry = drmGetEntry(fd);
 
-    return drmHashDelete(entry->tagTable, context);
+    if (entry)
+	return drmHashDelete(entry->tagTable, context);
+    return -ENOMEM;
 }
 
 void *drmGetContextTag(int fd, drm_context_t context)
 {
-    drmHashEntry  *entry = drmGetEntry(fd);
     void          *value;
+    drmHashEntry  *entry = drmGetEntry(fd);
+
+    if (!entry)
+        return NULL;
 
     if (drmHashLookup(entry->tagTable, context, &value))
 	return NULL;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 12/12] xf86drmSL: Add missing function call to SLLocate()

[Praveen Paneri]

A call to SLLocate() is missing from the function drmSLLookupNeighbors()
Adding the same to fix this bug.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 xf86drmSL.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xf86drmSL.c b/xf86drmSL.c
index edafe7b..9c6f65a 100644
--- a/xf86drmSL.c
+++ b/xf86drmSL.c
@@ -274,6 +274,7 @@ int drmSLLookupNeighbors(void *l, unsigned long key,
     SLEntryPtr    update[SL_MAX_LEVEL + 1];
     int           retcode = 0;
 
+    SLLocate(list, key, update);
     *prev_key   = *next_key   = key;
     *prev_value = *next_value = NULL;
 	
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/12] intel: Validate output of realloc()

[Chris Wilson]

On Thu, Apr 02, 2015 at 02:02:17PM +0530, Praveen Paneri wrote:
> realloc will return NULL if failed to allocate the extra memory
> requested. Return from function if it fails.

NAK. Silently passing absolute addresses to the GPU to read and write is
not a good idea.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx