Fix for static analysis issues

Below are the issues fixed: - Uninitialized scalar variable - Use after free - Resource leak - Explicit null dereferenced - Unused value - Rule of three - COPY_INSTEAD_OF_MOVE - Dereference after null check - Dereference null return value - Untrusted loop bound - Use after free - Missing assignment operator Tracked-On: OAM-122342 Signed-off-by: Sapna <[email protected]>
projectceladon · Aug 1, 2024 · 198f942 · 198f942
1 parent 567b233
commit 198f942
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 12 deletions.
diff --git a/cros_gralloc/aidl/Allocator.cpp b/cros_gralloc/aidl/Allocator.cpp
@@ -116,7 +116,7 @@ ndk::ScopedAStatus Allocator::allocate(const BufferDescriptorInfoV4& descriptor,
         return ToBinderStatus(AllocationError::NO_RESOURCES);
     }
 
-    struct cros_gralloc_buffer_descriptor crosDescriptor;
+    struct cros_gralloc_buffer_descriptor crosDescriptor = {};
     if (convertToCrosDescriptor(descriptor, &crosDescriptor)) {
         return ToBinderStatus(AllocationError::UNSUPPORTED);
     }
@@ -147,9 +147,11 @@ ndk::ScopedAStatus Allocator::allocate(const BufferDescriptorInfoV4& descriptor,
         releaseBufferAndHandle(handle);
         return status;
     }
-
-    *outStride = static_cast<int32_t>(crosHandle->pixel_stride);
-    *outHandle = handle;
+
+    if (crosHandle != nullptr) {
+        *outStride = static_cast<int32_t>(crosHandle->pixel_stride);
+        *outHandle = handle;
+    }
 
     return ndk::ScopedAStatus::ok();
 }
@@ -239,4 +241,4 @@ ::ndk::SpAIBinder Allocator::createBinder() {
     return binder;
 }
 
-}  // namespace aidl::android::hardware::graphics::allocator::impl
+}  // namespace aidl::android::hardware::graphics::allocator::impl
diff --git a/cros_gralloc/cros_gralloc_driver.cc b/cros_gralloc/cros_gralloc_driver.cc
@@ -34,6 +34,11 @@ class cros_gralloc_driver_preloader
 	{
 		drv_preload(true);
 	}
+	//Copy Constructor
+	cros_gralloc_driver_preloader(const cros_gralloc_driver_preloader&) = default;
+
+	//Copy Assignment Operator
+	cros_gralloc_driver_preloader& operator=(const cros_gralloc_driver_preloader&) = default;
 
 	~cros_gralloc_driver_preloader()
 	{
@@ -179,7 +184,7 @@ cros_gralloc_driver::cros_gralloc_driver()
 		switch (availabe_node) {
 		// only have one render node, is GVT-d/BM/VirtIO
 		case 1:
-			if (!drv_render_) {
+			if (drv_render_) {
 				fd = drv_get_fd(drv_render_);
 				drv_destroy(drv_render_);
 				close(fd);

diff --git a/cros_gralloc/cros_gralloc_driver.h b/cros_gralloc/cros_gralloc_driver.h
@@ -58,6 +58,10 @@ class cros_gralloc_driver
 
       private:
 	cros_gralloc_driver();
+	//Copy Constructor
+	cros_gralloc_driver(const cros_gralloc_driver&) = delete;
+	//Copy Assignment Operator
+	cros_gralloc_driver& operator=(const cros_gralloc_driver&) = delete;
 	~cros_gralloc_driver();
 	bool is_initialized();
 	cros_gralloc_buffer *get_buffer(cros_gralloc_handle_t hnd);

diff --git a/cros_gralloc/gralloc4/CrosGralloc4Mapper.cc b/cros_gralloc/gralloc4/CrosGralloc4Mapper.cc
@@ -968,7 +968,7 @@ Return<void> CrosGralloc4Mapper::dumpBuffer(const cros_gralloc_buffer* crosBuffe
     auto metadata_get_callback = [&](Error, hidl_vec<uint8_t> metadata) {
         MetadataDump metadataDump;
         metadataDump.metadataType = metadataType;
-        metadataDump.metadata = metadata;
+        metadataDump.metadata = std::move(metadata);
         metadataDumps.push_back(metadataDump);
     };
 

diff --git a/drv_helpers.c b/drv_helpers.c
@@ -522,11 +522,14 @@ void *drv_dumb_bo_map(struct bo *bo, struct vma *vma, uint32_t map_flags)
 
 int drv_bo_munmap(struct bo *bo, struct vma *vma)
 {
+	int ret = -1;
 	if (vma->cpu) {
 		free(vma->addr);
 		vma->addr = NULL;
 	}
-	return munmap(vma->addr, vma->length);
+	if (vma->addr != NULL)
+		ret = munmap(vma->addr, vma->length); 
+	return ret;
 }
 
 int drv_get_prot(uint32_t map_flags)

diff --git a/i915.c b/i915.c
@@ -492,8 +492,8 @@ static int i915_add_combinations(struct driver *drv)
 static int i915_align_dimensions(struct bo *bo, uint32_t format, uint32_t tiling, uint32_t *stride,
 				 uint32_t *aligned_height)
 {
-	uint32_t horizontal_alignment = 64;
-	uint32_t vertical_alignment = 4;
+	uint32_t horizontal_alignment;
+	uint32_t vertical_alignment;
 	struct i915_device *i915 = bo->drv->priv;
 	if (i915->graphics_version >= 125) {
 		horizontal_alignment = 4;

diff --git a/virtgpu_cross_domain.c b/virtgpu_cross_domain.c
@@ -18,6 +18,7 @@
 
 #define CAPSET_CROSS_DOMAIN 5
 #define CAPSET_CROSS_FAKE 30
+#define MAX_PLANES 4
 
 static const uint32_t scanout_render_formats[] = { DRM_FORMAT_ABGR8888, DRM_FORMAT_ARGB8888,
 						   DRM_FORMAT_RGB565, DRM_FORMAT_XBGR8888,
@@ -62,8 +63,6 @@ static void cross_domain_release_private(struct driver *drv)
 		drv_array_destroy(priv->metadata_cache);
 
 	pthread_mutex_destroy(&priv->metadata_cache_lock);
-
-	free(priv);
 }
 
 static void add_combinations(struct driver *drv)
@@ -350,6 +349,7 @@ static int cross_domain_init(struct driver *drv)
 
 free_private:
 	cross_domain_release_private(drv);
+	free(priv);
 	return ret;
 }
 
@@ -404,6 +404,10 @@ static int cross_domain_bo_create(struct bo *bo, uint32_t width, uint32_t height
 		return -errno;
 	}
 
+	if (bo->meta.num_planes <= 0 || bo->meta.num_planes > MAX_PLANES) {
+		return -1;
+	}
+
 	for (uint32_t plane = 0; plane < bo->meta.num_planes; plane++)
 		bo->handles[plane].u32 = drm_rc_blob.bo_handle;