On Mon, Apr 3, 2017 at 4:26 PM, Chad Versace <chadvers...@chromium.org>
wrote:
> On Wed 15 Mar 2017, Jason Ekstrand wrote:
> > This cache allows us to easily ensure that we have a unique anv_bo for
> > each gem handle. We'll need this in order to support multiple-import of
> > memory objects and semaphores.
> >
> > v2 (Jason Ekstrand):
> > - Reject BO imports if the size doesn't match the prime fd size as
> > reported by lseek().
> >
> > v3 (Jason Ekstrand):
> > - Fix reference counting around cache_release (Chris Willson)
> > - Move the mutex_unlock() later in cache_release
> > ---
> > src/intel/vulkan/anv_allocator.c | 261 ++++++++++++++++++++++++++++++
> +++++++++
> > src/intel/vulkan/anv_private.h | 26 ++++
> > 2 files changed, 287 insertions(+)
>
>
>
> > +VkResult
> > +anv_bo_cache_import(struct anv_device *device,
> > + struct anv_bo_cache *cache,
> > + int fd, uint64_t size, struct anv_bo **bo_out,
> > + VkAllocationCallbacks *alloc)
> > +{
> > + 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);
> > + return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHX);
> > + }
> > +
> > + struct anv_cached_bo *bo = anv_bo_cache_lookup_locked(cache,
> gem_handle);
> > + if (bo) {
> > + assert(bo->bo.size == size);
>
> For security's sake, we must always check the size, even if the bo is in
> the cache. An assertion isn't enough. A malicious client may tell the
> truth the first time when sending the fd to the trusted app. The
> malicious client may then dup the fd and send it to the trusted app with
> a false size.
>
Yup. Fixed locally.
--Jason
> > + __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) {
> > + anv_gem_close(device, gem_handle);
> > + pthread_mutex_unlock(&cache->mutex);
> > + return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHX);
> > + }
> > +
> > + struct anv_cached_bo *bo =
> > + vk_alloc(alloc, size, 8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
> > + if (!bo) {
> > + anv_gem_close(device, gem_handle);
> > + pthread_mutex_unlock(&cache->mutex);
> > + return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> > + }
> > +
> > + bo->refcount = 1;
> > +
> > + anv_bo_init(&bo->bo, gem_handle, size);
> > +
> > + _mesa_hash_table_insert(cache->bo_map, (void
> *)(uintptr_t)gem_handle, bo);
> > + }
> > +
> > + pthread_mutex_unlock(&cache->mutex);
> > +
> > + /* From the Vulkan spec:
> > + *
> > + * "Importing memory from a file descriptor transfers ownership of
> > + * the file descriptor from the application to the Vulkan
> > + * implementation. The application must not perform any
> operations on
> > + * the file descriptor after a successful import."
> > + *
> > + * If the import fails, we leave the file descriptor open.
> > + */
> > + close(fd);
> > +
> > + *bo_out = &bo->bo;
> > +
> > + return VK_SUCCESS;
> > +}
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
