From e9f3e8642692cf8c44988cfda495a03c79ee1e8b Mon Sep 17 00:00:00 2001 From: Sapna Date: Fri, 26 Jul 2024 01:35:20 +0530 Subject: [PATCH] Fix for static analysis issues Below are the issues fixed: - Buffer not null terminated - Resource leak - Logically dead code - Argument cannot be negative - Dead default in switch - Dereference after null check - Unchecked return value - Data race condition - Unchecked return value from library Tracked-On: OAM-122340 Signed-off-by: Sapna --- intel/intel_bufmgr_fake.c | 15 ++++++---- intel/intel_bufmgr_gem.c | 58 ++++++++++++++++++++++----------------- intel/intel_decode.c | 17 ++++++------ libsync.h | 9 +++++- xf86drm.c | 11 ++++++-- xf86drmSL.c | 1 + 6 files changed, 67 insertions(+), 44 deletions(-) diff --git a/intel/intel_bufmgr_fake.c b/intel/intel_bufmgr_fake.c index 0cec51f5b..47227d601 100644 --- a/intel/intel_bufmgr_fake.c +++ b/intel/intel_bufmgr_fake.c @@ -1170,9 +1170,11 @@ static int assert(bo_fake->map_count == 0); if (bo_fake->is_static) { - /* Add it to the needs-fence list */ - bufmgr_fake->need_fence = 1; - return 0; + /* Add it to the needs-fence list */ + pthread_mutex_lock(&bufmgr_fake->lock); + bufmgr_fake->need_fence = 1; + pthread_mutex_unlock(&bufmgr_fake->lock); + return 0; } /* Allocate the card memory */ @@ -1185,8 +1187,8 @@ static int assert(bo_fake->block); assert(bo_fake->block->bo == &bo_fake->bo); - - bo->offset = bo_fake->block->mem->ofs; + if (!bo_fake->block) + bo->offset = bo_fake->block->mem->ofs; /* Upload the buffer contents if necessary */ if (bo_fake->dirty) { @@ -1359,7 +1361,8 @@ drm_intel_fake_reloc_and_validate_buffer(drm_intel_bo *bo) if (bo->virtual == NULL) drm_intel_fake_bo_map_locked(bo, 1); - *(uint32_t *) ((uint8_t *) bo->virtual + r->offset) = + if (bo->virtual != NULL) + *(uint32_t *) ((uint8_t *) bo->virtual + r->offset) = reloc_data; r->last_target_offset = r->target_buf->offset; diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index f14bb9208..2d56ffb44 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -428,20 +428,23 @@ drm_intel_gem_dump_validation_list(drm_intel_bufmgr_gem *bufmgr_gem) drm_intel_bo *target_bo = bo_gem->reloc_target_info[j].bo; drm_intel_bo_gem *target_gem = (drm_intel_bo_gem *) target_bo; - - DBG("%2d: %d %s(%s)@0x%08x %08x -> " - "%d (%s)@0x%08x %08x + 0x%08x\n", - i, - bo_gem->gem_handle, - bo_gem->kflags & EXEC_OBJECT_PINNED ? "*" : "", - bo_gem->name, - upper_32_bits(bo_gem->relocs[j].offset), - lower_32_bits(bo_gem->relocs[j].offset), - target_gem->gem_handle, - target_gem->name, - upper_32_bits(target_bo->offset64), - lower_32_bits(target_bo->offset64), - bo_gem->relocs[j].delta); + if (bo_gem->relocs != NULL) { + DBG("%2d: %d %s(%s)@0x%08x %08x -> " + "%d (%s)@0x%08x %08x + 0x%08x\n", + i, + bo_gem->gem_handle, + bo_gem->kflags & EXEC_OBJECT_PINNED ? "*" : "", + bo_gem->name, + upper_32_bits(bo_gem->relocs[j].offset), + lower_32_bits(bo_gem->relocs[j].offset), + target_gem->gem_handle, + target_gem->name, + upper_32_bits(target_bo->offset64), + lower_32_bits(target_bo->offset64), + bo_gem->relocs[j].delta); + } else { + DBG("bo_gem->relocs is NULL for iteration number : %d", j); + } } for (j = 0; j < bo_gem->softpin_target_count; j++) { @@ -623,12 +626,15 @@ drm_intel_gem_bo_madvise_internal(drm_intel_bufmgr_gem *bufmgr_gem, drm_intel_bo_gem *bo_gem, int state) { struct drm_i915_gem_madvise madv; + int ret; memclear(madv); madv.handle = bo_gem->gem_handle; madv.madv = state; madv.retained = 1; - drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_MADVISE, &madv); + ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_MADVISE, &madv); + if (ret < 0) + return ret; return madv.retained; } @@ -976,7 +982,8 @@ has_userptr(drm_intel_bufmgr_gem *bufmgr_gem) pgsz = sysconf(_SC_PAGESIZE); assert(pgsz > 0); - + if (pgsz < 0) + return false; ret = posix_memalign(&ptr, pgsz, pgsz); if (ret) { DBG("Failed to get a page (%ld) for userptr detection!\n", @@ -2075,15 +2082,16 @@ do_bo_emit_reloc(drm_intel_bo *bo, uint32_t offset, else bo_gem->reloc_target_info[bo_gem->reloc_count].flags = 0; - bo_gem->relocs[bo_gem->reloc_count].offset = offset; - bo_gem->relocs[bo_gem->reloc_count].delta = target_offset; - bo_gem->relocs[bo_gem->reloc_count].target_handle = - target_bo_gem->gem_handle; - bo_gem->relocs[bo_gem->reloc_count].read_domains = read_domains; - bo_gem->relocs[bo_gem->reloc_count].write_domain = write_domain; - bo_gem->relocs[bo_gem->reloc_count].presumed_offset = target_bo->offset64; - bo_gem->reloc_count++; - + if (bo_gem->relocs != NULL) { + bo_gem->relocs[bo_gem->reloc_count].offset = offset; + bo_gem->relocs[bo_gem->reloc_count].delta = target_offset; + bo_gem->relocs[bo_gem->reloc_count].target_handle = + target_bo_gem->gem_handle; + bo_gem->relocs[bo_gem->reloc_count].read_domains = read_domains; + bo_gem->relocs[bo_gem->reloc_count].write_domain = write_domain; + bo_gem->relocs[bo_gem->reloc_count].presumed_offset = target_bo->offset64; + bo_gem->reloc_count++; + } return 0; } diff --git a/intel/intel_decode.c b/intel/intel_decode.c index b0fc2288d..63a97a7c1 100644 --- a/intel/intel_decode.c +++ b/intel/intel_decode.c @@ -656,8 +656,7 @@ i915_get_instruction_dst(uint32_t *data, int i, char *dstname, int do_mask) switch ((a0 >> 19) & 0x7) { case 0: - if (dst_nr > 15) - fprintf(out, "bad destination reg R%d\n", dst_nr); + //The value of dst_nr must be between 0 and 15. sprintf(dstname, "R%d%s%s", dst_nr, dstmask, sat); break; case 4: @@ -988,8 +987,9 @@ i915_decode_dcl(struct drm_intel_decode *ctx, int i, char *instr_prefix) sampletype = "RESERVED"; break; } - if (dcl_nr > 15) - fprintf(out, "bad S%d dcl register number\n", dcl_nr); + //The condition dcl_nr > 15 cannot be true. + //if (dcl_nr > 15) + // fprintf(out, "bad S%d dcl register number\n", dcl_nr); instr_out(ctx, i++, "%s: DCL S%d %s\n", instr_prefix, dcl_nr, sampletype); instr_out(ctx, i++, "%s\n", instr_prefix); @@ -1114,7 +1114,8 @@ decode_compare_func(uint32_t op) case 7: return "gequal"; } - return ""; + // Adding an assertion to indicate that this point should never be reached. + __builtin_unreachable(); } static const char * @@ -1138,7 +1139,8 @@ decode_stencil_op(uint32_t op) case 7: return "decr"; } - return ""; + // Adding an assertion to indicate that this point should never be reached. + __builtin_unreachable(); } #if 0 @@ -3835,9 +3837,6 @@ drm_intel_decode_context_alloc(uint32_t devid) /* Just assume future unknown platforms behave as gen8. */ gen = 8; - if (!gen) - return NULL; - ctx = calloc(1, sizeof(struct drm_intel_decode)); if (!ctx) return NULL; diff --git a/libsync.h b/libsync.h index f1a2f96d3..dea2e5348 100644 --- a/libsync.h +++ b/libsync.h @@ -89,7 +89,14 @@ static inline int sync_merge(const char *name, int fd1, int fd2) int ret; data.fd2 = fd2; - strncpy(data.name, name, sizeof(data.name)); + + if (sizeof(data.name) < strlen(name)) { + strncpy(data.name, name, sizeof(data.name)); + data.name[sizeof(data.name) - 1] = '\0'; + } else { + strncpy(data.name, name, strlen(name) - 1); + data.name[strlen(name)] = '\0'; + } do { ret = ioctl(fd1, SYNC_IOC_MERGE, &data); diff --git a/xf86drm.c b/xf86drm.c index 3bb024ec1..d2964d36a 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -882,7 +882,10 @@ static int drmOpenDevice(dev_t dev, int minor, int type) return DRM_ERR_NOT_ROOT; mkdir(DRM_DIR_NAME, DRM_DEV_DIRMODE); chown_check_return(DRM_DIR_NAME, 0, 0); /* root:root */ - chmod(DRM_DIR_NAME, DRM_DEV_DIRMODE); + if (chmod(DRM_DIR_NAME, DRM_DEV_DIRMODE) != 0) { + // If chmod returns -1, an error occurred + return errno; + } } /* Check if the device node exists and create it if necessary. */ @@ -896,7 +899,8 @@ static int drmOpenDevice(dev_t dev, int minor, int type) if (drm_server_info && drm_server_info->get_perms) { group = ((int)serv_group >= 0) ? serv_group : DRM_DEV_GID; chown_check_return(buf, user, group); - chmod(buf, devmode); + if (chmod(buf, devmode) != 0) + return errno; } #else /* if we modprobed then wait for udev */ @@ -940,7 +944,8 @@ static int drmOpenDevice(dev_t dev, int minor, int type) mknod(buf, S_IFCHR | devmode, dev); if (drm_server_info && drm_server_info->get_perms) { chown_check_return(buf, user, group); - chmod(buf, devmode); + if (chmod(buf, devmode) != 0) + return errno; } } fd = open(buf, O_RDWR | O_CLOEXEC); diff --git a/xf86drmSL.c b/xf86drmSL.c index 3826df97f..f05f864b5 100644 --- a/xf86drmSL.c +++ b/xf86drmSL.c @@ -182,6 +182,7 @@ drm_public int drmSLInsert(void *l, unsigned long key, void *value) } ++list->count; + free(entry); return 0; /* Added to table */ }