Skip to content

Commit

Permalink
Fix for static analysis issues
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@Sapna1-singh
Sapna1-singh committed Jul 25, 2024
1 parent 26942cf commit 3b98e1f
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 43 deletions.
15 changes: 9 additions & 6 deletions intel/intel_bufmgr_fake.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
58 changes: 33 additions & 25 deletions intel/intel_bufmgr_gem.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand Down Expand Up @@ -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 false;

return madv.retained;
}
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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;
}

Expand Down
22 changes: 13 additions & 9 deletions intel/intel_decode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -656,8 +656,9 @@ 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.
//if (dst_nr > 15)
// fprintf(out, "bad destination reg R%d\n", dst_nr);
sprintf(dstname, "R%d%s%s", dst_nr, dstmask, sat);
break;
case 4:
Expand Down Expand Up @@ -988,8 +989,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);
Expand Down Expand Up @@ -1114,7 +1116,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 *
Expand All @@ -1138,7 +1141,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
Expand Down Expand Up @@ -3834,9 +3838,9 @@ drm_intel_decode_context_alloc(uint32_t devid)
else
/* Just assume future unknown platforms behave as gen8. */
gen = 8;

if (!gen)
return NULL;
// LOGICALLY_DEAD_CODE: As the value of gen can't be 0.
//if (!gen)
// return NULL;

ctx = calloc(1, sizeof(struct drm_intel_decode));
if (!ctx)
Expand Down
5 changes: 5 additions & 0 deletions libsync.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ static inline int sync_merge(const char *name, int fd1, int fd2)
data.fd2 = fd2;
strncpy(data.name, name, sizeof(data.name));

if (sizeof(data.name) < strlen(name))
data.name[sizeof(data.name) - 1] = '\0';
else
data.name[strlen(name)] = '\0';

do {
ret = ioctl(fd1, SYNC_IOC_MERGE, &data);
} while (ret == -1 && (errno == EINTR || errno == EAGAIN));
Expand Down
11 changes: 8 additions & 3 deletions xf86drm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions xf86drmSL.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ drm_public int drmSLInsert(void *l, unsigned long key, void *value)
}

++list->count;
free(entry);
return 0; /* Added to table */
}

Expand Down

0 comments on commit 3b98e1f

Please sign in to comment.