[DynInst_API:] [PATCH] Address some issues found by coverity

[William Cohen <wcohen@xxxxxxxxxx>]

Hi All,

I have been looking over the coverity scan for the most recent release
of dyninst-8.1.1 and found some errors that could be easily corrected.
Attached are the patches and the descriptions of the coverity errors
each patch addresses.

-Will



dyninst-rtheap1.patch fixes:

Error: RESOURCE_LEAK (CWE-772):
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:113: open_fn: Returning handle opened by function "open(char const *, int, ...)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:113: var_assign: Assigning: "fd" = handle returned from "open("/proc/self/maps", 0)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:114: cond_false: Condition "0 > fd", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:117: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:119: cond_true: Condition "1", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:121: noescape: Resource "fd" is not freed or pointed-to in function "read(int, void *, size_t)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:122: cond_false: Condition "0 == ret", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:122: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:123: cond_false: Condition "0 > ret", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:126: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:128: cond_true: Condition "length >= 32768UL /* sizeof (procAsciiMap) */", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:130: leaked_handle: Handle variable "fd" going out of scope leaks the handle.

Error: RESOURCE_LEAK (CWE-772):
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:113: open_fn: Returning handle opened by function "open(char const *, int, ...)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:113: var_assign: Assigning: "fd" = handle returned from "open("/proc/self/maps", 0)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:114: cond_false: Condition "0 > fd", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:117: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:119: cond_true: Condition "1", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:121: noescape: Resource "fd" is not freed or pointed-to in function "read(int, void *, size_t)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:122: cond_false: Condition "0 == ret", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:122: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:123: cond_false: Condition "0 > ret", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:126: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:128: cond_false: Condition "length >= 32768UL /* sizeof (procAsciiMap) */", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:131: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:132: loop: Jumping back to the beginning of the loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:119: loop_begin: Jumped back to beginning of loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:119: cond_true: Condition "1", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:121: noescape: Resource "fd" is not freed or pointed-to in function "read(int, void *, size_t)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:122: cond_false: Condition "0 == ret", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:122: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:123: cond_true: Condition "0 > ret", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:125: leaked_handle: Handle variable "fd" going out of scope leaks the handle.


Patch5: dyninst-rtheap2.patch  fixes:

Error: RESOURCE_LEAK (CWE-772):
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:252: alloc_fn: Storage is returned from allocation function "malloc(size_t)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:252: var_assign: Assigning: "node" = storage returned from "malloc(48UL)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:255: cond_true: Condition "psize == -1", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:258: cond_true: Condition "size % DYNINSTheap_align != 0", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:258: leaked_storage: Variable "node" going out of scope leaks the storage it points to.

Error: RESOURCE_LEAK (CWE-772):
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:252: alloc_fn: Storage is returned from allocation function "malloc(size_t)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:252: var_assign: Assigning: "node" = storage returned from "malloc(48UL)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:255: cond_true: Condition "psize == -1", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:258: cond_false: Condition "size % DYNINSTheap_align != 0", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:258: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:261: cond_false: Condition "DYNINSTheap_useMalloc(lo_addr, hi_addr)", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:293: else_branch: Reached else branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:304: cond_true: Condition "hi < DYNINSTheap_loAddr", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:308: leaked_storage: Variable "node" going out of scope leaks the storage it points to.

Error: RESOURCE_LEAK (CWE-772):
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:255: cond_true: Condition "psize == -1", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:258: cond_false: Condition "size % DYNINSTheap_align != 0", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:258: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:261: cond_false: Condition "DYNINSTheap_useMalloc(lo_addr, hi_addr)", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:293: else_branch: Reached else branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:304: cond_false: Condition "hi < DYNINSTheap_loAddr", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:304: cond_false: Condition "lo > DYNINSTheap_hiAddr", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:309: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:314: alloc_arg: "DYNINSTgetMemoryMap(unsigned int *, dyninstmm_t **)" allocates memory that is stored into "maps".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:114:4: cond_false: Condition "0 > fd", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:117:4: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:119:4: cond_true: Condition "1", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:122:7: cond_true: Condition "0 == ret", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:122:21: break: Breaking from loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:132:4: loop_end: Reached end of loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:139:4: cond_true: Condition "p != NULL", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:142:7: loop: Jumping back to the beginning of the loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:139:4: loop_begin: Jumped back to beginning of loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:139:4: cond_true: Condition "p != NULL", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:142:7: loop: Jumping back to the beginning of the loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:139:4: loop_begin: Jumped back to beginning of loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:139:4: cond_false: Condition "p != NULL", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:142:7: loop_end: Reached end of loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:144:4: alloc_fn: Storage is returned from allocation function "malloc(size_t)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:144:4: var_assign: Assigning: "ms" = "malloc(num * 16UL)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:145:4: cond_false: Condition "!ms", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:148:4: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:151:4: cond_true: Condition "i < num", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:157:7: cond_false: Condition "!p", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:157:16: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:163:7: cond_false: Condition "!p", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:163:16: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:170:4: loop: Jumping back to the beginning of the loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:151:4: loop_begin: Jumped back to beginning of loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:151:4: cond_true: Condition "i < num", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:157:7: cond_false: Condition "!p", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:157:16: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:163:7: cond_false: Condition "!p", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:163:16: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:170:4: loop: Jumping back to the beginning of the loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:151:4: loop_begin: Jumped back to beginning of loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:151:4: cond_false: Condition "i < num", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:170:4: loop_end: Reached end of loop
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c:173:4: var_assign: Assigning: "*mapp" = "ms".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:314: cond_false: Condition "0 > DYNINSTgetMemoryMap(&nmaps, &maps)", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:320: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:321: noescape: Resource "maps" is not freed or pointed-to in function "qsort(void *, size_t, size_t, __compar_fn_t)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:322: noescape: Resource "maps" is not freed or pointed-to in function "heap_checkMappings(int, dyninstmm_t *)".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:94:56: noescape: "heap_checkMappings(int, dyninstmm_t *)" does not free or save its pointer parameter "maps".
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:328: cond_true: Condition "0 > fd", taking true branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c:330: leaked_storage: Variable "maps" going out of scope leaks the storage it points to.


dyninst-cov3.patch fixes:

Error: SIZECHECK (CWE-131):
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTthread.c:71: cond_false: Condition "init_index_done", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTthread.c:71: if_end: End of if statement
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTthread.c:74: cond_false: Condition "DYNINST_max_num_threads == 32", taking false branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTthread.c:79: else_branch: Reached else branch
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTthread.c:81: buffer_alloc: "malloc(DYNINST_thread_hash_size * 4UL)" allocates memory.
dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTthread.c:81: incorrect_multiplication: Allocating a multiple of 4 bytes to pointer of type long, which needs 8 bytes.    free(maps);



I don't have a patch, but the following error also looks like it should be fixed addressed in the dyninst source:

Error: SIZECHECK (CWE-131):
dyninst-8.1.1/dyninst/parseThat/src/parseThat.C:203: cond_true: Condition "pid == 0", taking true branch
dyninst-8.1.1/dyninst/parseThat/src/parseThat.C:207: cond_true: Condition "config.use_exe", taking true branch
dyninst-8.1.1/dyninst/parseThat/src/parseThat.C:209: if_fallthrough: Falling through to end of if statement
dyninst-8.1.1/dyninst/parseThat/src/parseThat.C:211: if_end: End of if statement
dyninst-8.1.1/dyninst/parseThat/src/parseThat.C:213: buffer_alloc: "malloc(2UL)" allocates memory.
dyninst-8.1.1/dyninst/parseThat/src/parseThat.C:213: size_error: Allocating 2 bytes to pointer of type char *, which needs at least 8 bytes.

diff -up dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c.cov dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c
--- dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c.cov	2013-03-21 12:11:57.602774174 -0400
+++ dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap-linux.c	2013-03-21 12:12:18.440826708 -0400
@@ -121,11 +121,13 @@ DYNINSTgetMemoryMap(unsigned *nump, dyni
       ret = read(fd, procAsciiMap + length, sizeof(procAsciiMap) - length);
       if (0 == ret) break;
       if (0 > ret) {
+	      close(fd);
 	      perror("read /proc");
 	      return -1;
       }
       length += ret;
       if (length >= sizeof(procAsciiMap)) {
+	      close(fd);
 	      fprintf(stderr, "DYNINSTgetMemoryMap: memory map buffer overflow\n");
 	      return -1;
       }

diff -up dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c.cov2 dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c
--- dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c.cov2	2013-03-21 12:16:13.829459860 -0400
+++ dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTheap.c	2013-03-21 12:22:51.027491819 -0400
@@ -255,7 +255,10 @@ void *DYNINSTos_malloc(size_t nbytes, vo
   if (psize == -1) psize = getpagesize();
 
   /* buffer size must be aligned */
-  if (size % DYNINSTheap_align != 0) return ((void *)-1);
+  if (size % DYNINSTheap_align != 0) {
+    free(node);
+    return ((void *)-1);
+  }
 
   /* use malloc() if appropriate */
   if (DYNINSTheap_useMalloc(lo_addr, hi_addr)) {
@@ -302,6 +305,7 @@ void *DYNINSTos_malloc(size_t nbytes, vo
    DYNINSTheap_loAddr = getpagesize();
 #endif
     if ((hi < DYNINSTheap_loAddr) || (lo > DYNINSTheap_hiAddr)) {
+      free(node);
 #ifdef DEBUG
       fprintf(stderr, "CAN'T MMAP IN RANGE GIVEN\n");
 #endif 
@@ -327,6 +331,7 @@ void *DYNINSTos_malloc(size_t nbytes, vo
 #if !defined(os_osf)
     if (0 > fd) {
       free(node);
+      free(maps);
       return NULL;
     }
 #endif

diff -up dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTthread.c.cov3 dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTthread.c
--- dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTthread.c.cov3	2013-03-21 12:38:11.019832212 -0400
+++ dyninst-8.1.1/dyninst/dyninstAPI_RT/src/RTthread.c	2013-03-21 12:38:21.403858624 -0400
@@ -79,7 +79,7 @@ void DYNINST_initialize_index_list()
   else {
      DYNINST_thread_hash_size = (int) (DYNINST_max_num_threads * 1.25);
      DYNINST_thread_hash_indices =  
-         malloc(DYNINST_thread_hash_size * sizeof(int));
+         malloc(DYNINST_thread_hash_size * sizeof(long));
 	DYNINST_thread_hash_tids = 
 		malloc(DYNINST_thread_hash_size * sizeof(dyntid_t));
   }