Browse Source

msm: camera: memmgr: Avoid TOCTOU buffer access during multiple use of same fd

Fd is a user-accessible value, referring it multiple times
leads to TOCTOU issues. Dma_buf can be freed after the 1st
use of fd and userspace can create another dma_buf but with
same fd. In such scenario, during 2nd use of fd, we may get
a different dma_buf with different length. To avoid this, we
can use same dma_buf instead of retrieving it twice using same
fd. In this change FD is accessed only once in syscall.

CRs-Fixed: 3159446
Change-Id: I00eb6dd3d798165f5c6c0bd59feabe80a68592b1
Signed-off-by: Yash Upadhyay <[email protected]>
Yash Upadhyay 3 years ago
parent
commit
86a7e8992a
2 changed files with 7 additions and 33 deletions
  1. 0 6
      drivers/cam_req_mgr/cam_mem_mgr.c
  2. 7 27
      drivers/cam_smmu/cam_smmu_api.c

+ 0 - 6
drivers/cam_req_mgr/cam_mem_mgr.c

@@ -1400,12 +1400,6 @@ static int cam_mem_util_unmap(int32_t idx,
 		if (cam_mem_util_unmap_hw_va(idx, region, client))
 			CAM_ERR(CAM_MEM, "Failed, dmabuf=%pK",
 				tbl.bufq[idx].dma_buf);
-		/*
-		 * Workaround as smmu driver doing put_buf without get_buf for kernel mappings
-		 * Setting NULL here so that we dont call dma_buf_pt again below
-		 */
-		if (client == CAM_SMMU_MAPPING_KERNEL)
-			tbl.bufq[idx].dma_buf = NULL;
 	}
 
 	mutex_lock(&tbl.m_lock);

+ 7 - 27
drivers/cam_smmu/cam_smmu_api.c

@@ -325,7 +325,7 @@ static struct cam_dma_buff_info *cam_smmu_find_mapping_by_virt_address(int idx,
 static int cam_smmu_map_buffer_and_add_to_list(int idx, int ion_fd,
 	bool dis_delayed_unmap, enum dma_data_direction dma_dir,
 	dma_addr_t *paddr_ptr, size_t *len_ptr,
-	enum cam_smmu_region_id region_id, bool is_internal);
+	enum cam_smmu_region_id region_id, bool is_internal, struct dma_buf *dmabuf);
 
 static int cam_smmu_map_kernel_buffer_and_add_to_list(int idx,
 	struct dma_buf *buf, enum dma_data_direction dma_dir,
@@ -2168,7 +2168,7 @@ static int cam_smmu_map_buffer_validate(struct dma_buf *buf,
 	if (IS_ERR_OR_NULL(attach)) {
 		rc = PTR_ERR(attach);
 		CAM_ERR(CAM_SMMU, "Error: dma buf attach failed");
-		goto err_put;
+		goto err_out;
 	}
 
 	if (region_id == CAM_SMMU_REGION_SHARED) {
@@ -2315,8 +2315,6 @@ err_unmap_sg:
 	dma_buf_unmap_attachment(attach, table, dma_dir);
 err_detach:
 	dma_buf_detach(buf, attach);
-err_put:
-	dma_buf_put(buf);
 err_out:
 	return rc;
 }
@@ -2325,14 +2323,10 @@ err_out:
 static int cam_smmu_map_buffer_and_add_to_list(int idx, int ion_fd,
 	bool dis_delayed_unmap, enum dma_data_direction dma_dir,
 	dma_addr_t *paddr_ptr, size_t *len_ptr,
-	enum cam_smmu_region_id region_id, bool is_internal)
+	enum cam_smmu_region_id region_id, bool is_internal, struct dma_buf *buf)
 {
 	int rc = -1;
 	struct cam_dma_buff_info *mapping_info = NULL;
-	struct dma_buf *buf = NULL;
-
-	/* returns the dma_buf structure related to an fd */
-	buf = dma_buf_get(ion_fd);
 
 	rc = cam_smmu_map_buffer_validate(buf, idx, dma_dir, paddr_ptr, len_ptr,
 		region_id, dis_delayed_unmap, &mapping_info);
@@ -2464,7 +2458,6 @@ static int cam_smmu_unmap_buf_and_remove_from_list(
 
 
 	dma_buf_detach(mapping_info->buf, mapping_info->attach);
-	dma_buf_put(mapping_info->buf);
 
 	if (iommu_cb_set.debug_cfg.map_profile_enable) {
 		CAM_GET_TIMESTAMP(ts2);
@@ -2977,10 +2970,9 @@ handle_err:
 
 static int cam_smmu_map_stage2_buffer_and_add_to_list(int idx, int ion_fd,
 		 enum dma_data_direction dma_dir, dma_addr_t *paddr_ptr,
-		 size_t *len_ptr)
+		 size_t *len_ptr, struct dma_buf *dmabuf)
 {
 	int rc = 0;
-	struct dma_buf *dmabuf = NULL;
 	struct dma_buf_attachment *attach = NULL;
 	struct sg_table *table = NULL;
 	struct cam_sec_buff_info *mapping_info;
@@ -2989,15 +2981,6 @@ static int cam_smmu_map_stage2_buffer_and_add_to_list(int idx, int ion_fd,
 	*paddr_ptr = (dma_addr_t)NULL;
 	*len_ptr = (size_t)0;
 
-	dmabuf = dma_buf_get(ion_fd);
-	if (IS_ERR_OR_NULL((void *)(dmabuf))) {
-		CAM_ERR(CAM_SMMU,
-			"Error: dma buf get failed, idx=%d, ion_fd=%d",
-			idx, ion_fd);
-		rc = PTR_ERR(dmabuf);
-		goto err_out;
-	}
-
 	/*
 	 * ion_phys() is deprecated. call dma_buf_attach() and
 	 * dma_buf_map_attachment() to get the buffer's physical
@@ -3009,7 +2992,7 @@ static int cam_smmu_map_stage2_buffer_and_add_to_list(int idx, int ion_fd,
 			"Error: dma buf attach failed, idx=%d, ion_fd=%d",
 			idx, ion_fd);
 		rc = PTR_ERR(attach);
-		goto err_put;
+		goto err_out;
 	}
 
 	attach->dma_map_attrs |= DMA_ATTR_SKIP_CPU_SYNC;
@@ -3056,8 +3039,6 @@ err_unmap_sg:
 	dma_buf_unmap_attachment(attach, table, dma_dir);
 err_detach:
 	dma_buf_detach(dmabuf, attach);
-err_put:
-	dma_buf_put(dmabuf);
 err_out:
 	return rc;
 }
@@ -3122,7 +3103,7 @@ int cam_smmu_map_stage2_iova(int handle, int ion_fd, struct dma_buf *dmabuf,
 		goto get_addr_end;
 	}
 	rc = cam_smmu_map_stage2_buffer_and_add_to_list(idx, ion_fd, dma_dir,
-			paddr_ptr, len_ptr);
+			paddr_ptr, len_ptr, dmabuf);
 	if (rc < 0) {
 		CAM_ERR(CAM_SMMU,
 			"Error: mapping or add list fail, idx=%d, handle=%d, fd=%d, rc=%d",
@@ -3158,7 +3139,6 @@ static int cam_smmu_secure_unmap_buf_and_remove_from_list(
 	dma_buf_unmap_attachment(mapping_info->attach,
 		mapping_info->table, mapping_info->dir);
 	dma_buf_detach(mapping_info->buf, mapping_info->attach);
-	dma_buf_put(mapping_info->buf);
 	mapping_info->buf = NULL;
 
 	list_del_init(&mapping_info->list);
@@ -3348,7 +3328,7 @@ int cam_smmu_map_user_iova(int handle, int ion_fd, struct dma_buf *dmabuf,
 
 	rc = cam_smmu_map_buffer_and_add_to_list(idx, ion_fd,
 		dis_delayed_unmap, dma_dir, paddr_ptr, len_ptr,
-		region_id, is_internal);
+		region_id, is_internal, dmabuf);
 	if (rc < 0) {
 		CAM_ERR(CAM_SMMU,
 			"mapping or add list fail cb:%s idx=%d, fd=%d, region=%d, rc=%d",