[PATCH 1/2] drm/etnaviv: take etnaviv_gem_obj in etnaviv_gem_mmap_obj

[PATCH 1/2] drm/etnaviv: take etnaviv_gem_obj in etnaviv_gem_mmap_obj

[Lucas Stach]

This function will be changed to be called indirectly and this
prototype change brings it in line with all the other indirect
object calls.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 281c6eca20a8..4d2277f6de03 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -129,10 +129,9 @@ void etnaviv_gem_put_pages(struct etnaviv_gem_object *etnaviv_obj)
 	/* when we start tracking the pin count, then do something here */
 }
 
-static int etnaviv_gem_mmap_obj(struct drm_gem_object *obj,
+static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
 		struct vm_area_struct *vma)
 {
-	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
 	pgprot_t vm_page_prot;
 
 	vma->vm_flags &= ~VM_PFNMAP;
@@ -151,9 +150,9 @@ static int etnaviv_gem_mmap_obj(struct drm_gem_object *obj,
 		 * in particular in the case of mmap'd dmabufs)
 		 */
 		fput(vma->vm_file);
-		get_file(obj->filp);
+		get_file(etnaviv_obj->base.filp);
 		vma->vm_pgoff = 0;
-		vma->vm_file  = obj->filp;
+		vma->vm_file  = etnaviv_obj->base.filp;
 
 		vma->vm_page_prot = vm_page_prot;
 	}
@@ -173,7 +172,7 @@ int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	}
 
 	obj = to_etnaviv_bo(vma->vm_private_data);
-	return etnaviv_gem_mmap_obj(vma->vm_private_data, vma);
+	return etnaviv_gem_mmap_obj(obj, vma);
 }
 
 int etnaviv_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
-- 
2.8.0.rc3

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

[PATCH 2/2] drm/etnaviv: fix mmap operations for userptr and dma-buf objects

[Lucas Stach]

Add an indirect object operations call to allow distinct implementations
of the mmap operation based on the type of the object.

This ensures that the exporter is called to set up the mmap for imported
dma-bufs and disallows mapping of userptr objects through the DRM file,
as this might lead to serious corruption of kernel internal state.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 10 +++++++++-
 drivers/gpu/drm/etnaviv/etnaviv_gem.h       |  1 +
 drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c |  7 +++++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 4d2277f6de03..1f919f5ca978 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -172,7 +172,7 @@ int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	}
 
 	obj = to_etnaviv_bo(vma->vm_private_data);
-	return etnaviv_gem_mmap_obj(obj, vma);
+	return obj->ops->mmap(obj, vma);
 }
 
 int etnaviv_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
@@ -544,6 +544,7 @@ static const struct etnaviv_gem_ops etnaviv_gem_shmem_ops = {
 	.get_pages = etnaviv_gem_shmem_get_pages,
 	.release = etnaviv_gem_shmem_release,
 	.vmap = etnaviv_gem_vmap_impl,
+	.mmap = etnaviv_gem_mmap_obj,
 };
 
 void etnaviv_gem_free_object(struct drm_gem_object *obj)
@@ -885,10 +886,17 @@ static void etnaviv_gem_userptr_release(struct etnaviv_gem_object *etnaviv_obj)
 	put_task_struct(etnaviv_obj->userptr.task);
 }
 
+static int etnaviv_gem_userptr_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
+		struct vm_area_struct *vma)
+{
+	return -EPERM;
+}
+
 static const struct etnaviv_gem_ops etnaviv_gem_userptr_ops = {
 	.get_pages = etnaviv_gem_userptr_get_pages,
 	.release = etnaviv_gem_userptr_release,
 	.vmap = etnaviv_gem_vmap_impl,
+	.mmap = etnaviv_gem_userptr_mmap_obj,
 };
 
 int etnaviv_gem_new_userptr(struct drm_device *dev, struct drm_file *file,
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index 02665d8c10ee..e63ff116a3b3 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -79,6 +79,7 @@ struct etnaviv_gem_ops {
 	int (*get_pages)(struct etnaviv_gem_object *);
 	void (*release)(struct etnaviv_gem_object *);
 	void *(*vmap)(struct etnaviv_gem_object *);
+	int (*mmap)(struct etnaviv_gem_object *, struct vm_area_struct *);
 };
 
 static inline bool is_active(struct etnaviv_gem_object *etnaviv_obj)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index 4e67395f5fa1..b93618c1aa69 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -84,10 +84,17 @@ static void *etnaviv_gem_prime_vmap_impl(struct etnaviv_gem_object *etnaviv_obj)
 	return dma_buf_vmap(etnaviv_obj->base.import_attach->dmabuf);
 }
 
+static int etnaviv_gem_prime_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
+		struct vm_area_struct *vma)
+{
+	return dma_buf_mmap(etnaviv_obj->base.dma_buf, vma, 0);
+}
+
 static const struct etnaviv_gem_ops etnaviv_gem_prime_ops = {
 	/* .get_pages should never be called */
 	.release = etnaviv_gem_prime_release,
 	.vmap = etnaviv_gem_prime_vmap_impl,
+	.mmap = etnaviv_gem_prime_mmap_obj,
 };
 
 struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
-- 
2.8.0.rc3

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

Re: [PATCH 2/2] drm/etnaviv: fix mmap operations for userptr and dma-buf objects

[Russell King - ARM Linux]

On Wed, Apr 27, 2016 at 02:39:21PM +0200, Lucas Stach wrote:
> +static int etnaviv_gem_userptr_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
> +		struct vm_area_struct *vma)
> +{
> +	return -EPERM;
> +}

       EPERM  The prot argument asks for PROT_EXEC but the mapped area belongs
              to a file on a file system that was mounted no-exec.

It seems this error code is not suitable for this application - it
doesn't fit the spirit of EPERM.  The universal EINVAL would probably
be better.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/etnaviv: take etnaviv_gem_obj in etnaviv_gem_mmap_obj

[Russell King - ARM Linux]

On Wed, Apr 27, 2016 at 02:39:20PM +0200, Lucas Stach wrote:
> This function will be changed to be called indirectly and this
> prototype change brings it in line with all the other indirect
> object calls.

Christian prefers passing around struct drm_gem_object rather than
the etnaviv_gem_object.  I'm much more in favour of passing the
appropriate data type like the patch below.

Please sort it out with Christian.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/etnaviv: fix mmap operations for userptr and dma-buf objects

[Lucas Stach]

Am Mittwoch, den 27.04.2016, 21:27 +0100 schrieb Russell King - ARM
Linux:
> On Wed, Apr 27, 2016 at 02:39:21PM +0200, Lucas Stach wrote:
> > +static int etnaviv_gem_userptr_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
> > +		struct vm_area_struct *vma)
> > +{
> > +	return -EPERM;
> > +}
> 
>        EPERM  The prot argument asks for PROT_EXEC but the mapped area belongs
>               to a file on a file system that was mounted no-exec.
> 
> It seems this error code is not suitable for this application - it
> doesn't fit the spirit of EPERM.  The universal EINVAL would probably
> be better.
> 
I've changed this to return EINVAL.

Regards,
Lucas

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