Re: [PATCH 5/7] drm/vgem: prime export support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ben,
Please refer to comments below.

On 02/22/2012 08:29 PM, Ben Widawsky wrote:
dma-buf export implementation. Heavily influenced by Dave Airlie's proof
of concept work.

Cc: Daniel Vetter<daniel.vetter@xxxxxxxx>
Cc: Dave Airlie<airlied@xxxxxxxxxx>
Signed-off-by: Ben Widawsky<ben@xxxxxxxxxxxx>
---
  drivers/gpu/drm/vgem/Makefile       |    2 +-
  drivers/gpu/drm/vgem/vgem_dma_buf.c |  128 +++++++++++++++++++++++++++++++++++
  drivers/gpu/drm/vgem/vgem_drv.c     |    6 ++
  drivers/gpu/drm/vgem/vgem_drv.h     |    7 ++
  4 files changed, 142 insertions(+), 1 deletions(-)
  create mode 100644 drivers/gpu/drm/vgem/vgem_dma_buf.c

[snip]
+
+int vgem_prime_to_fd(struct drm_device *dev, struct drm_file *file,
+		     uint32_t handle, int *prime_fd)
+{
+	struct drm_vgem_file_private *file_priv = file->driver_priv;
+	struct drm_vgem_gem_object *obj;
+	int ret;
+
+	DRM_DEBUG_PRIME("Request fd for handle %d\n", handle);
+
+	obj = to_vgem_bo(drm_gem_object_lookup(dev, file, handle));
+	if (!obj)
+		return -EBADF;
+
+	/* This means a user has already called get_fd on this */
+	if (obj->base.prime_fd != -1) {
+		DRM_DEBUG_PRIME("User requested a previously exported buffer "
+				"%d %d\n", handle, obj->base.prime_fd);
+		drm_gem_object_unreference(&obj->base);
+		goto out_fd;

IMO, it is not safe to cache a number of file descriptor. This number may unexpectedly become invalid introducing a bug. Please refer to the scenario:
- application possess a GEM buffer called A
- call DRM_IOCTL_PRIME_HANDLE_TO_FD on A to obtain a file descriptor fd_A
- application calls fd_B = dup(fd_A). Now both fd_A and fd_B point to the same DMABUF instance. - application calls close(fd_A), now fd_A is no longer a valid file descriptor - application tries to export the buffer again and calls ioctl DRM_IOCTL_PRIME_HANDLE_TO_FD on GEM buffer. The field obj->base.prime_fd is already set to fd_A so value fd_A is returned to userspace. Notice that this is a bug because fd_A is no longer valid.

I think that field prime_fd should be removed because it is too unreliable. The driver should call dma_buf_fd every time when the buffer is exported.

+	}
+
+	/* Make a dma buf out of our vgem object */
+	obj->base.export_dma_buf = dma_buf_export(obj,&vgem_dmabuf_ops,
+						  obj->base.size,
+						  VGEM_FD_PERMS);
+	if (IS_ERR(obj->base.export_dma_buf)) {
+		DRM_DEBUG_PRIME("export fail\n");
+		return PTR_ERR(obj->base.export_dma_buf);
+	} else
+		obj->base.prime_fd = dma_buf_fd(obj->base.export_dma_buf);
+
+	mutex_lock(&dev->prime_mutex);
+	ret = drm_prime_insert_fd_handle_mapping(&file_priv->prime,
+						 obj->base.export_dma_buf,
+						 handle);
+	WARN_ON(ret);
+	ret = drm_prime_add_dma_buf(dev,&obj->base);
+	mutex_unlock(&dev->prime_mutex);
+	if (ret)
+		return ret;
+
+out_fd:
+	*prime_fd = obj->base.prime_fd;
+
+	return 0;
+}
+

Regards,
Tomasz Stanislawski

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux