[Mesa-dev] [PATCH v3 07/18] anv/allocator: Add a BO cache

[Jason Ekstrand]

On Mon, Apr 3, 2017 at 4:26 PM, Chad Versace <chadversary at 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;
> > +}
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170403/a984d6eb/attachment.html>