anv: Move size check from anv_bo_cache_import() to caller (v2)
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, but it does change behavior
in some edge cases, described below. In what follows, the "nominal size"
of the bo refers to anv_bo::size, which may not match the bo's "actual
size" according to the kernel.
Post-patch, the nominal size of the bo returned from
anv_bo_cache_import() is always the size of imported dma-buf according
to lseek(). Pre-patch, the bo's nominal size was difficult to predict.
If the imported dma-buf's gem handle was not resident in the cache, then
the bo's nominal size was align(VkMemoryAllocateInfo::allocationSize,
4096). If it *was* resident, then the bo's nominal size was whatever
the cache returned. As a consequence, the first cache insert decided the
bo's nominal size, which could be significantly smaller compared to the
dma-buf's actual size, as the nominal size was determined by
VkMemoryAllocationInfo::allocationSize and not lseek().
I believe this patch cleans up that messy behavior. For an imported or
exported VkDeviceMemory, anv_bo::size should now be the true size of the
bo, if I correctly understand the problem (which I possibly don't).
v2:
- Preserve behavior of aligning size to 4096 before checking. [for
jekstrand]
- Check size with < instead of <=, to match behavior of commit c0a4f56
"anv: bo_cache: allow importing a BO larger than needed". [for
chadv]
This commit is contained in:
parent
fbf39fd7c3
commit
9775894f10
|
@ -1272,13 +1272,10 @@ anv_bo_cache_alloc(struct anv_device *device,
|
||||||
VkResult
|
VkResult
|
||||||
anv_bo_cache_import(struct anv_device *device,
|
anv_bo_cache_import(struct anv_device *device,
|
||||||
struct anv_bo_cache *cache,
|
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);
|
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);
|
uint32_t gem_handle = anv_gem_fd_to_handle(device, fd);
|
||||||
if (!gem_handle) {
|
if (!gem_handle) {
|
||||||
pthread_mutex_unlock(&cache->mutex);
|
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);
|
struct anv_cached_bo *bo = anv_bo_cache_lookup_locked(cache, gem_handle);
|
||||||
if (bo) {
|
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);
|
__sync_fetch_and_add(&bo->refcount, 1);
|
||||||
} else {
|
} else {
|
||||||
/* For security purposes, we reject BO imports where the size does not
|
off_t size = lseek(fd, 0, SEEK_END);
|
||||||
* match exactly. This prevents a malicious client from passing a
|
if (size == (off_t)-1) {
|
||||||
* 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) {
|
|
||||||
anv_gem_close(device, gem_handle);
|
anv_gem_close(device, gem_handle);
|
||||||
pthread_mutex_unlock(&cache->mutex);
|
pthread_mutex_unlock(&cache->mutex);
|
||||||
return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR);
|
return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR);
|
||||||
|
|
|
@ -1543,11 +1543,32 @@ VkResult anv_AllocateMemory(
|
||||||
VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHR);
|
VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHR);
|
||||||
|
|
||||||
result = anv_bo_cache_import(device, &device->bo_cache,
|
result = anv_bo_cache_import(device, &device->bo_cache,
|
||||||
fd_info->fd, pAllocateInfo->allocationSize,
|
fd_info->fd, &mem->bo);
|
||||||
&mem->bo);
|
|
||||||
if (result != VK_SUCCESS)
|
if (result != VK_SUCCESS)
|
||||||
goto fail;
|
goto fail;
|
||||||
|
|
||||||
|
VkDeviceSize aligned_alloc_size =
|
||||||
|
align_u64(pAllocateInfo->allocationSize, 4096);
|
||||||
|
|
||||||
|
/* For security purposes, we reject importing the bo if it's smaller
|
||||||
|
* than the requested allocation size. 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 (mem->bo->size < aligned_alloc_size) {
|
||||||
|
result = vk_errorf(device->instace, device,
|
||||||
|
VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR,
|
||||||
|
"aligned allocationSize too large for "
|
||||||
|
"VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHR: "
|
||||||
|
"%"PRIu64"B > %"PRIu64"B",
|
||||||
|
aligned_alloc_size, mem->bo->size);
|
||||||
|
anv_bo_cache_release(device, &device->bo_cache, mem->bo);
|
||||||
|
goto fail;
|
||||||
|
}
|
||||||
|
|
||||||
/* From the Vulkan spec:
|
/* From the Vulkan spec:
|
||||||
*
|
*
|
||||||
* "Importing memory from a file descriptor transfers ownership of
|
* "Importing memory from a file descriptor transfers ownership of
|
||||||
|
|
|
@ -76,10 +76,22 @@ VkResult anv_CreateDmaBufImageINTEL(
|
||||||
image = anv_image_from_handle(image_h);
|
image = anv_image_from_handle(image_h);
|
||||||
|
|
||||||
result = anv_bo_cache_import(device, &device->bo_cache,
|
result = anv_bo_cache_import(device, &device->bo_cache,
|
||||||
pCreateInfo->fd, image->size, &mem->bo);
|
pCreateInfo->fd, &mem->bo);
|
||||||
if (result != VK_SUCCESS)
|
if (result != VK_SUCCESS)
|
||||||
goto fail_import;
|
goto fail_import;
|
||||||
|
|
||||||
|
VkDeviceSize aligned_image_size = align_u64(image->size, 4096);
|
||||||
|
|
||||||
|
if (mem->bo->size < aligned_image_size) {
|
||||||
|
result = vk_errorf(device->instace, device,
|
||||||
|
VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR,
|
||||||
|
"dma-buf too small for image in "
|
||||||
|
"vkCreateDmaBufImageINTEL: %"PRIu64"B < "PRIu64"B",
|
||||||
|
mem->bo->size, aligned_image_size);
|
||||||
|
anv_bo_cache_release(device, &device->bo_cache, mem->bo);
|
||||||
|
goto fail_import;
|
||||||
|
}
|
||||||
|
|
||||||
if (device->instance->physicalDevice.supports_48bit_addresses)
|
if (device->instance->physicalDevice.supports_48bit_addresses)
|
||||||
mem->bo->flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
|
mem->bo->flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
|
||||||
|
|
||||||
|
|
|
@ -718,7 +718,7 @@ VkResult anv_bo_cache_alloc(struct anv_device *device,
|
||||||
uint64_t size, struct anv_bo **bo);
|
uint64_t size, struct anv_bo **bo);
|
||||||
VkResult anv_bo_cache_import(struct anv_device *device,
|
VkResult anv_bo_cache_import(struct anv_device *device,
|
||||||
struct anv_bo_cache *cache,
|
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,
|
VkResult anv_bo_cache_export(struct anv_device *device,
|
||||||
struct anv_bo_cache *cache,
|
struct anv_bo_cache *cache,
|
||||||
struct anv_bo *bo_in, int *fd_out);
|
struct anv_bo *bo_in, int *fd_out);
|
||||||
|
|
|
@ -1023,10 +1023,15 @@ VkResult anv_ImportSemaphoreFdKHR(
|
||||||
new_impl.type = ANV_SEMAPHORE_TYPE_BO;
|
new_impl.type = ANV_SEMAPHORE_TYPE_BO;
|
||||||
|
|
||||||
VkResult result = anv_bo_cache_import(device, &device->bo_cache,
|
VkResult result = anv_bo_cache_import(device, &device->bo_cache,
|
||||||
fd, 4096, &new_impl.bo);
|
fd, &new_impl.bo);
|
||||||
if (result != VK_SUCCESS)
|
if (result != VK_SUCCESS)
|
||||||
return result;
|
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
|
/* If we're going to use this as a fence, we need to *not* have the
|
||||||
* EXEC_OBJECT_ASYNC bit set.
|
* EXEC_OBJECT_ASYNC bit set.
|
||||||
*/
|
*/
|
||||||
|
|
Loading…
Reference in New Issue