| Message ID | 1415792295-2466-2-git-send-email-daniel.thompson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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. Thank you, Damian > >> --- >> 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 >>
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!
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;
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(-)