[Mesa-dev] [PATCH 07/10] anv: Move size check from anv_bo_cache_import() to caller
Chad Versace
chadversary at chromium.org
Wed Oct 18 00:27:04 UTC 2017
This change prepares for VK_ANDROID_native_buffer. When the user imports
a gralloc hande into a VkImage using VK_ANDROID_native_buffer, the user
provides no size. The driver must infer the size from the internals of
the gralloc buffer.
The patch is essentially a refactor patch. It should change no behavior
other than improved debug messages.
---
Jason, you were right about the race; it poses no security problem.
This patch replaces 07/10 in the series.
src/intel/vulkan/anv_allocator.c | 21 +++------------------
src/intel/vulkan/anv_device.c | 21 +++++++++++++++++++--
src/intel/vulkan/anv_intel.c | 12 +++++++++++-
src/intel/vulkan/anv_private.h | 2 +-
src/intel/vulkan/anv_queue.c | 7 ++++++-
5 files changed, 40 insertions(+), 23 deletions(-)
diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_allocator.c
index 401cea40e6..27eedb53aa 100644
--- a/src/intel/vulkan/anv_allocator.c
+++ b/src/intel/vulkan/anv_allocator.c
@@ -1272,13 +1272,10 @@ anv_bo_cache_alloc(struct anv_device *device,
VkResult
anv_bo_cache_import(struct anv_device *device,
struct anv_bo_cache *cache,
- int fd, uint64_t size, struct anv_bo **bo_out)
+ int fd, struct anv_bo **bo_out)
{
pthread_mutex_lock(&cache->mutex);
- /* The kernel is going to give us whole pages anyway */
- size = align_u64(size, 4096);
-
uint32_t gem_handle = anv_gem_fd_to_handle(device, fd);
if (!gem_handle) {
pthread_mutex_unlock(&cache->mutex);
@@ -1287,22 +1284,10 @@ anv_bo_cache_import(struct anv_device *device,
struct anv_cached_bo *bo = anv_bo_cache_lookup_locked(cache, gem_handle);
if (bo) {
- if (bo->bo.size != size) {
- pthread_mutex_unlock(&cache->mutex);
- return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR);
- }
__sync_fetch_and_add(&bo->refcount, 1);
} else {
- /* For security purposes, we reject BO imports where the size does not
- * match exactly. This prevents a malicious client from passing a
- * buffer to a trusted client, lying about the size, and telling the
- * trusted client to try and texture from an image that goes
- * out-of-bounds. This sort of thing could lead to GPU hangs or worse
- * in the trusted client. The trusted client can protect itself against
- * this sort of attack but only if it can trust the buffer size.
- */
- off_t import_size = lseek(fd, 0, SEEK_END);
- if (import_size == (off_t)-1 || import_size < size) {
+ off_t size = lseek(fd, 0, SEEK_END);
+ if (size == (off_t)-1) {
anv_gem_close(device, gem_handle);
pthread_mutex_unlock(&cache->mutex);
return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR);
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index 1634b5158c..900de9778c 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -1543,11 +1543,28 @@ VkResult anv_AllocateMemory(
VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHR);
result = anv_bo_cache_import(device, &device->bo_cache,
- fd_info->fd, pAllocateInfo->allocationSize,
- &mem->bo);
+ fd_info->fd, &mem->bo);
if (result != VK_SUCCESS)
goto fail;
+ /* For security purposes, we reject BO imports where the size does not
+ * match exactly. This prevents a malicious client from passing
+ * a buffer to a trusted client, lying about the size, and telling the
+ * trusted client to try and texture from an image that goes
+ * out-of-bounds. This sort of thing could lead to GPU hangs or worse
+ * in the trusted client. The trusted client can protect itself against
+ * this sort of attack but only if it can trust the buffer size.
+ */
+ if (pAllocateInfo->allocationSize != mem->bo->size) {
+ result = vk_errorf(device->instace, device,
+ VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR,
+ "invalid allocationSize for VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHR: "
+ "expected %"PRIu64"B, actual %"PRIu64"B",
+ mem->bo->size, pAllocateInfo->allocationSize);
+ anv_bo_cache_release(device, &device->bo_cache, mem->bo);
+ goto fail;
+ }
+
/* From the Vulkan spec:
*
* "Importing memory from a file descriptor transfers ownership of
diff --git a/src/intel/vulkan/anv_intel.c b/src/intel/vulkan/anv_intel.c
index d6bad44091..a80b1e4623 100644
--- a/src/intel/vulkan/anv_intel.c
+++ b/src/intel/vulkan/anv_intel.c
@@ -76,10 +76,20 @@ VkResult anv_CreateDmaBufImageINTEL(
image = anv_image_from_handle(image_h);
result = anv_bo_cache_import(device, &device->bo_cache,
- pCreateInfo->fd, image->size, &mem->bo);
+ pCreateInfo->fd, &mem->bo);
if (result != VK_SUCCESS)
goto fail_import;
+ if (image->size != mem->bo->size) {
+ result = vk_errorf(device->instace, device,
+ VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR,
+ "invalid dma-buf size in vkCreateDmaBufImageINTEL: "
+ "expected %"PRIu64"B, actual %"PRIu64"B",
+ image->size, mem->bo->size);
+ anv_bo_cache_release(device, &device->bo_cache, mem->bo);
+ goto fail_import;
+ }
+
if (device->instance->physicalDevice.supports_48bit_addresses)
mem->bo->flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index 8af3f5c69e..61e23282b0 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -718,7 +718,7 @@ VkResult anv_bo_cache_alloc(struct anv_device *device,
uint64_t size, struct anv_bo **bo);
VkResult anv_bo_cache_import(struct anv_device *device,
struct anv_bo_cache *cache,
- int fd, uint64_t size, struct anv_bo **bo);
+ int fd, struct anv_bo **bo);
VkResult anv_bo_cache_export(struct anv_device *device,
struct anv_bo_cache *cache,
struct anv_bo *bo_in, int *fd_out);
diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c
index 7e675e2274..63335490fd 100644
--- a/src/intel/vulkan/anv_queue.c
+++ b/src/intel/vulkan/anv_queue.c
@@ -1023,10 +1023,15 @@ VkResult anv_ImportSemaphoreFdKHR(
new_impl.type = ANV_SEMAPHORE_TYPE_BO;
VkResult result = anv_bo_cache_import(device, &device->bo_cache,
- fd, 4096, &new_impl.bo);
+ fd, &new_impl.bo);
if (result != VK_SUCCESS)
return result;
+ if (new_impl.bo->size != 4096) {
+ anv_bo_cache_release(device, &device->bo_cache, new_impl.bo);
+ return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR);
+ }
+
/* If we're going to use this as a fence, we need to *not* have the
* EXEC_OBJECT_ASYNC bit set.
*/
--
2.13.0
More information about the mesa-dev
mailing list