diff mbox

[v2,v3.18-rc4,1/4] drm: prime: Honour O_RDWR during prime-handle-to-fd

Message ID 1415792295-2466-2-git-send-email-daniel.thompson@linaro.org
State New
Headers show

Commit Message

Daniel Thompson Nov. 12, 2014, 11:38 a.m. UTC
Currently DRM_IOCTL_PRIME_HANDLE_TO_FD rejects all flags except
(DRM|O)_CLOEXEC making it hard for the userspace to generate a file
descriptor that can be used by mmap().

It is easy to relax the restriction and allow read/write permissions.
This should be safe because the flags are seldom touched by drm; mostly
they are passed verbatim to dma_buf calls.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 drivers/gpu/drm/drm_prime.c | 9 +++------
 include/uapi/drm/drm.h      | 1 +
 2 files changed, 4 insertions(+), 6 deletions(-)

Comments

Rob Clark Nov. 12, 2014, 2:41 p.m. UTC | #1
On Wed, Nov 12, 2014 at 6:38 AM, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
> Currently DRM_IOCTL_PRIME_HANDLE_TO_FD rejects all flags except
> (DRM|O)_CLOEXEC making it hard for the userspace to generate a file
> descriptor that can be used by mmap().
>
> It is easy to relax the restriction and allow read/write permissions.
> This should be safe because the flags are seldom touched by drm; mostly
> they are passed verbatim to dma_buf calls.
>
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>

Reviewed-by: Rob Clark <robdclark@gmail.com>

> ---
>  drivers/gpu/drm/drm_prime.c | 9 +++------
>  include/uapi/drm/drm.h      | 1 +
>  2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 78ca30808422..8467b17c8053 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -331,7 +331,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
>   * drm_gem_prime_export - helper library implemention of the export callback
>   * @dev: drm_device to export from
>   * @obj: GEM object to export
> - * @flags: flags like DRM_CLOEXEC
> + * @flags: flags like DRM_CLOEXEC and DRM_RDWR
>   *
>   * This is the implementation of the gem_prime_export functions for GEM drivers
>   * using the PRIME helpers.
> @@ -635,14 +635,11 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
>                 return -ENOSYS;
>
>         /* check flags are valid */
> -       if (args->flags & ~DRM_CLOEXEC)
> +       if (args->flags & ~(DRM_CLOEXEC | DRM_RDWR))
>                 return -EINVAL;
>
> -       /* we only want to pass DRM_CLOEXEC which is == O_CLOEXEC */
> -       flags = args->flags & DRM_CLOEXEC;
> -
>         return dev->driver->prime_handle_to_fd(dev, file_priv,
> -                       args->handle, flags, &args->fd);
> +                       args->handle, args->flags, &args->fd);
>  }
>
>  int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index b0b855613641..89c2b68ddc51 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -660,6 +660,7 @@ struct drm_set_client_cap {
>         __u64 value;
>  };
>
> +#define DRM_RDWR O_RDWR
>  #define DRM_CLOEXEC O_CLOEXEC
>  struct drm_prime_handle {
>         __u32 handle;
> --
> 1.9.3
>
Daniel Thompson May 27, 2015, 7:48 a.m. UTC | #2
On 27/05/15 04:15, Damian Hobson-Garcia wrote:
> Hello,
>> On Wed, Nov 12, 2014 at 6:38 AM, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>>> Currently DRM_IOCTL_PRIME_HANDLE_TO_FD rejects all flags except
>>> (DRM|O)_CLOEXEC making it hard for the userspace to generate a file
>>> descriptor that can be used by mmap().
>>>
>>> It is easy to relax the restriction and allow read/write permissions.
>>> This should be safe because the flags are seldom touched by drm; mostly
>>> they are passed verbatim to dma_buf calls.
>>>
>>> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
>>
>> Reviewed-by: Rob Clark <robdclark@gmail.com>
>
> It's a little bit old by now, but I'm wondering if someone call tell me
> whether this patch is likely to be merged sometime, or has it been
> (should it be?) abandoned.

For me, this code remains useful and it would be good to merge it.

I accidentally removed the whole patchset from my 
"keep-resending-these-patches" when patch 3 went upstream... I'll rebase 
this when I get a  chance.


Daniel.


PS
Damn it all..., Rob may (or may not) remember my saying I had trouble 
getting the DRM GFX code to come up on my Android/IFC6410 port. Losing 
track of this patch would certainly explain it!

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 78ca30808422..8467b17c8053 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -331,7 +331,7 @@  static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
  * drm_gem_prime_export - helper library implemention of the export callback
  * @dev: drm_device to export from
  * @obj: GEM object to export
- * @flags: flags like DRM_CLOEXEC
+ * @flags: flags like DRM_CLOEXEC and DRM_RDWR
  *
  * This is the implementation of the gem_prime_export functions for GEM drivers
  * using the PRIME helpers.
@@ -635,14 +635,11 @@  int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
 		return -ENOSYS;
 
 	/* check flags are valid */
-	if (args->flags & ~DRM_CLOEXEC)
+	if (args->flags & ~(DRM_CLOEXEC | DRM_RDWR))
 		return -EINVAL;
 
-	/* we only want to pass DRM_CLOEXEC which is == O_CLOEXEC */
-	flags = args->flags & DRM_CLOEXEC;
-
 	return dev->driver->prime_handle_to_fd(dev, file_priv,
-			args->handle, flags, &args->fd);
+			args->handle, args->flags, &args->fd);
 }
 
 int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index b0b855613641..89c2b68ddc51 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -660,6 +660,7 @@  struct drm_set_client_cap {
 	__u64 value;
 };
 
+#define DRM_RDWR O_RDWR
 #define DRM_CLOEXEC O_CLOEXEC
 struct drm_prime_handle {
 	__u32 handle;