* [PATCH] drm/rockchip: use drm_gem_mmap helpers @ 2015-07-07 9:03 ` Daniel Kurtz 0 siblings, 0 replies; 8+ messages in thread From: Daniel Kurtz @ 2015-07-07 9:03 UTC (permalink / raw) Cc: Kees Cook, Daniel Vetter, open list:DRM DRIVERS FOR ROCKCHIP, Douglas Anderson, stable, open list, open list:ARM/Rockchip SoC support, moderated list:ARM/Rockchip SoC support Rather than (incompletely [0]) re-implementing drm_gem_mmap() and drm_gem_mmap_obj() helpers, call them directly from the rockchip mmap routines. Once the core functions return successfully, the rockchip mmap routines can still use dma_mmap_attrs() to simply mmap the entire buffer. [0] Previously, we were performing the mmap() without first taking a reference on the underlying gem buffer. This could leak ptes if the gem object is destroyed while userspace is still holding the mapping. Signed-off-by: Daniel Kurtz <djkurtz@chromium.org> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: stable@vger.kernel.org --- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 67 +++++++++++++++-------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index eb2282c..eba5f8a 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -54,55 +54,56 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj) &rk_obj->dma_attrs); } -int rockchip_gem_mmap_buf(struct drm_gem_object *obj, - struct vm_area_struct *vma) +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj, + struct vm_area_struct *vma) + { + int ret; struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); struct drm_device *drm = obj->dev; - unsigned long vm_size; - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; - vm_size = vma->vm_end - vma->vm_start; - - if (vm_size > obj->size) - return -EINVAL; + /* + * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear + * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap(). + */ + vma->vm_flags &= ~VM_PFNMAP; - return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, + ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, obj->size, &rk_obj->dma_attrs); + if (ret) + drm_gem_vm_close(vma); + + return ret; } -/* drm driver mmap file operations */ -int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) +int rockchip_gem_mmap_buf(struct drm_gem_object *obj, + struct vm_area_struct *vma) { - struct drm_file *priv = filp->private_data; - struct drm_device *dev = priv->minor->dev; - struct drm_gem_object *obj; - struct drm_vma_offset_node *node; + struct drm_device *drm = obj->dev; int ret; - if (drm_device_is_unplugged(dev)) - return -ENODEV; + mutex_lock(&drm->struct_mutex); + ret = drm_gem_mmap_obj(obj, obj->size, vma); + mutex_unlock(&drm->struct_mutex); + if (ret) + return ret; - mutex_lock(&dev->struct_mutex); + return rockchip_drm_gem_object_mmap(obj, vma); +} - node = drm_vma_offset_exact_lookup(dev->vma_offset_manager, - vma->vm_pgoff, - vma_pages(vma)); - if (!node) { - mutex_unlock(&dev->struct_mutex); - DRM_ERROR("failed to find vma node.\n"); - return -EINVAL; - } else if (!drm_vma_node_is_allowed(node, filp)) { - mutex_unlock(&dev->struct_mutex); - return -EACCES; - } +/* drm driver mmap file operations */ +int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) +{ + struct drm_gem_object *obj; + int ret; - obj = container_of(node, struct drm_gem_object, vma_node); - ret = rockchip_gem_mmap_buf(obj, vma); + ret = drm_gem_mmap(filp, vma); + if (ret) + return ret; - mutex_unlock(&dev->struct_mutex); + obj = vma->vm_private_data; - return ret; + return rockchip_drm_gem_object_mmap(obj, vma); } struct rockchip_gem_object * -- 2.4.3.573.g4eafbef _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] drm/rockchip: use drm_gem_mmap helpers @ 2015-07-07 9:03 ` Daniel Kurtz 0 siblings, 0 replies; 8+ messages in thread From: Daniel Kurtz @ 2015-07-07 9:03 UTC (permalink / raw) Cc: Daniel Vetter, Kees Cook, Douglas Anderson, Daniel Kurtz, stable, Mark Yao, David Airlie, Heiko Stuebner, open list:DRM DRIVERS FOR ROCKCHIP, moderated list:ARM/Rockchip SoC support, open list:ARM/Rockchip SoC support, open list Rather than (incompletely [0]) re-implementing drm_gem_mmap() and drm_gem_mmap_obj() helpers, call them directly from the rockchip mmap routines. Once the core functions return successfully, the rockchip mmap routines can still use dma_mmap_attrs() to simply mmap the entire buffer. [0] Previously, we were performing the mmap() without first taking a reference on the underlying gem buffer. This could leak ptes if the gem object is destroyed while userspace is still holding the mapping. Signed-off-by: Daniel Kurtz <djkurtz@chromium.org> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: stable@vger.kernel.org --- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 67 +++++++++++++++-------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index eb2282c..eba5f8a 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -54,55 +54,56 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj) &rk_obj->dma_attrs); } -int rockchip_gem_mmap_buf(struct drm_gem_object *obj, - struct vm_area_struct *vma) +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj, + struct vm_area_struct *vma) + { + int ret; struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); struct drm_device *drm = obj->dev; - unsigned long vm_size; - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; - vm_size = vma->vm_end - vma->vm_start; - - if (vm_size > obj->size) - return -EINVAL; + /* + * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear + * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap(). + */ + vma->vm_flags &= ~VM_PFNMAP; - return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, + ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, obj->size, &rk_obj->dma_attrs); + if (ret) + drm_gem_vm_close(vma); + + return ret; } -/* drm driver mmap file operations */ -int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) +int rockchip_gem_mmap_buf(struct drm_gem_object *obj, + struct vm_area_struct *vma) { - struct drm_file *priv = filp->private_data; - struct drm_device *dev = priv->minor->dev; - struct drm_gem_object *obj; - struct drm_vma_offset_node *node; + struct drm_device *drm = obj->dev; int ret; - if (drm_device_is_unplugged(dev)) - return -ENODEV; + mutex_lock(&drm->struct_mutex); + ret = drm_gem_mmap_obj(obj, obj->size, vma); + mutex_unlock(&drm->struct_mutex); + if (ret) + return ret; - mutex_lock(&dev->struct_mutex); + return rockchip_drm_gem_object_mmap(obj, vma); +} - node = drm_vma_offset_exact_lookup(dev->vma_offset_manager, - vma->vm_pgoff, - vma_pages(vma)); - if (!node) { - mutex_unlock(&dev->struct_mutex); - DRM_ERROR("failed to find vma node.\n"); - return -EINVAL; - } else if (!drm_vma_node_is_allowed(node, filp)) { - mutex_unlock(&dev->struct_mutex); - return -EACCES; - } +/* drm driver mmap file operations */ +int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) +{ + struct drm_gem_object *obj; + int ret; - obj = container_of(node, struct drm_gem_object, vma_node); - ret = rockchip_gem_mmap_buf(obj, vma); + ret = drm_gem_mmap(filp, vma); + if (ret) + return ret; - mutex_unlock(&dev->struct_mutex); + obj = vma->vm_private_data; - return ret; + return rockchip_drm_gem_object_mmap(obj, vma); } struct rockchip_gem_object * -- 2.4.3.573.g4eafbef ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] drm/rockchip: use drm_gem_mmap helpers @ 2015-07-07 9:03 ` Daniel Kurtz 0 siblings, 0 replies; 8+ messages in thread From: Daniel Kurtz @ 2015-07-07 9:03 UTC (permalink / raw) To: linux-arm-kernel Rather than (incompletely [0]) re-implementing drm_gem_mmap() and drm_gem_mmap_obj() helpers, call them directly from the rockchip mmap routines. Once the core functions return successfully, the rockchip mmap routines can still use dma_mmap_attrs() to simply mmap the entire buffer. [0] Previously, we were performing the mmap() without first taking a reference on the underlying gem buffer. This could leak ptes if the gem object is destroyed while userspace is still holding the mapping. Signed-off-by: Daniel Kurtz <djkurtz@chromium.org> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: stable at vger.kernel.org --- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 67 +++++++++++++++-------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index eb2282c..eba5f8a 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -54,55 +54,56 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj) &rk_obj->dma_attrs); } -int rockchip_gem_mmap_buf(struct drm_gem_object *obj, - struct vm_area_struct *vma) +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj, + struct vm_area_struct *vma) + { + int ret; struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); struct drm_device *drm = obj->dev; - unsigned long vm_size; - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; - vm_size = vma->vm_end - vma->vm_start; - - if (vm_size > obj->size) - return -EINVAL; + /* + * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear + * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap(). + */ + vma->vm_flags &= ~VM_PFNMAP; - return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, + ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, obj->size, &rk_obj->dma_attrs); + if (ret) + drm_gem_vm_close(vma); + + return ret; } -/* drm driver mmap file operations */ -int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) +int rockchip_gem_mmap_buf(struct drm_gem_object *obj, + struct vm_area_struct *vma) { - struct drm_file *priv = filp->private_data; - struct drm_device *dev = priv->minor->dev; - struct drm_gem_object *obj; - struct drm_vma_offset_node *node; + struct drm_device *drm = obj->dev; int ret; - if (drm_device_is_unplugged(dev)) - return -ENODEV; + mutex_lock(&drm->struct_mutex); + ret = drm_gem_mmap_obj(obj, obj->size, vma); + mutex_unlock(&drm->struct_mutex); + if (ret) + return ret; - mutex_lock(&dev->struct_mutex); + return rockchip_drm_gem_object_mmap(obj, vma); +} - node = drm_vma_offset_exact_lookup(dev->vma_offset_manager, - vma->vm_pgoff, - vma_pages(vma)); - if (!node) { - mutex_unlock(&dev->struct_mutex); - DRM_ERROR("failed to find vma node.\n"); - return -EINVAL; - } else if (!drm_vma_node_is_allowed(node, filp)) { - mutex_unlock(&dev->struct_mutex); - return -EACCES; - } +/* drm driver mmap file operations */ +int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) +{ + struct drm_gem_object *obj; + int ret; - obj = container_of(node, struct drm_gem_object, vma_node); - ret = rockchip_gem_mmap_buf(obj, vma); + ret = drm_gem_mmap(filp, vma); + if (ret) + return ret; - mutex_unlock(&dev->struct_mutex); + obj = vma->vm_private_data; - return ret; + return rockchip_drm_gem_object_mmap(obj, vma); } struct rockchip_gem_object * -- 2.4.3.573.g4eafbef ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/rockchip: use drm_gem_mmap helpers 2015-07-07 9:03 ` Daniel Kurtz (?) @ 2015-07-07 12:04 ` Daniel Vetter -1 siblings, 0 replies; 8+ messages in thread From: Daniel Vetter @ 2015-07-07 12:04 UTC (permalink / raw) To: Daniel Kurtz Cc: Kees Cook, Daniel Vetter, Douglas Anderson, stable, open list, open list:ARM/Rockchip SoC support, open list:DRM DRIVERS FOR ROCKCHIP, moderated list:ARM/Rockchip SoC support On Tue, Jul 07, 2015 at 05:03:36PM +0800, Daniel Kurtz wrote: > Rather than (incompletely [0]) re-implementing drm_gem_mmap() and > drm_gem_mmap_obj() helpers, call them directly from the rockchip mmap > routines. > > Once the core functions return successfully, the rockchip mmap routines > can still use dma_mmap_attrs() to simply mmap the entire buffer. > > [0] Previously, we were performing the mmap() without first taking a > reference on the underlying gem buffer. This could leak ptes if the gem > object is destroyed while userspace is still holding the mapping. > > Signed-off-by: Daniel Kurtz <djkurtz@chromium.org> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: stable@vger.kernel.org Applied to topic/drm-fixes to make sure it won't get lost, but I expect rockchip maintainers to pick this one up. -Daniel > > --- > drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 67 +++++++++++++++-------------- > 1 file changed, 34 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > index eb2282c..eba5f8a 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > @@ -54,55 +54,56 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj) > &rk_obj->dma_attrs); > } > > -int rockchip_gem_mmap_buf(struct drm_gem_object *obj, > - struct vm_area_struct *vma) > +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj, > + struct vm_area_struct *vma) > + > { > + int ret; > struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); > struct drm_device *drm = obj->dev; > - unsigned long vm_size; > > - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; > - vm_size = vma->vm_end - vma->vm_start; > - > - if (vm_size > obj->size) > - return -EINVAL; > + /* > + * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear > + * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap(). > + */ > + vma->vm_flags &= ~VM_PFNMAP; > > - return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, > + ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, > obj->size, &rk_obj->dma_attrs); > + if (ret) > + drm_gem_vm_close(vma); > + > + return ret; > } > > -/* drm driver mmap file operations */ > -int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) > +int rockchip_gem_mmap_buf(struct drm_gem_object *obj, > + struct vm_area_struct *vma) > { > - struct drm_file *priv = filp->private_data; > - struct drm_device *dev = priv->minor->dev; > - struct drm_gem_object *obj; > - struct drm_vma_offset_node *node; > + struct drm_device *drm = obj->dev; > int ret; > > - if (drm_device_is_unplugged(dev)) > - return -ENODEV; > + mutex_lock(&drm->struct_mutex); > + ret = drm_gem_mmap_obj(obj, obj->size, vma); > + mutex_unlock(&drm->struct_mutex); > + if (ret) > + return ret; > > - mutex_lock(&dev->struct_mutex); > + return rockchip_drm_gem_object_mmap(obj, vma); > +} > > - node = drm_vma_offset_exact_lookup(dev->vma_offset_manager, > - vma->vm_pgoff, > - vma_pages(vma)); > - if (!node) { > - mutex_unlock(&dev->struct_mutex); > - DRM_ERROR("failed to find vma node.\n"); > - return -EINVAL; > - } else if (!drm_vma_node_is_allowed(node, filp)) { > - mutex_unlock(&dev->struct_mutex); > - return -EACCES; > - } > +/* drm driver mmap file operations */ > +int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) > +{ > + struct drm_gem_object *obj; > + int ret; > > - obj = container_of(node, struct drm_gem_object, vma_node); > - ret = rockchip_gem_mmap_buf(obj, vma); > + ret = drm_gem_mmap(filp, vma); > + if (ret) > + return ret; > > - mutex_unlock(&dev->struct_mutex); > + obj = vma->vm_private_data; > > - return ret; > + return rockchip_drm_gem_object_mmap(obj, vma); > } > > struct rockchip_gem_object * > -- > 2.4.3.573.g4eafbef > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/rockchip: use drm_gem_mmap helpers @ 2015-07-07 12:04 ` Daniel Vetter 0 siblings, 0 replies; 8+ messages in thread From: Daniel Vetter @ 2015-07-07 12:04 UTC (permalink / raw) To: Daniel Kurtz Cc: Daniel Vetter, Kees Cook, Douglas Anderson, stable, Mark Yao, David Airlie, Heiko Stuebner, open list:DRM DRIVERS FOR ROCKCHIP, moderated list:ARM/Rockchip SoC support, open list:ARM/Rockchip SoC support, open list On Tue, Jul 07, 2015 at 05:03:36PM +0800, Daniel Kurtz wrote: > Rather than (incompletely [0]) re-implementing drm_gem_mmap() and > drm_gem_mmap_obj() helpers, call them directly from the rockchip mmap > routines. > > Once the core functions return successfully, the rockchip mmap routines > can still use dma_mmap_attrs() to simply mmap the entire buffer. > > [0] Previously, we were performing the mmap() without first taking a > reference on the underlying gem buffer. This could leak ptes if the gem > object is destroyed while userspace is still holding the mapping. > > Signed-off-by: Daniel Kurtz <djkurtz@chromium.org> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: stable@vger.kernel.org Applied to topic/drm-fixes to make sure it won't get lost, but I expect rockchip maintainers to pick this one up. -Daniel > > --- > drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 67 +++++++++++++++-------------- > 1 file changed, 34 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > index eb2282c..eba5f8a 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > @@ -54,55 +54,56 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj) > &rk_obj->dma_attrs); > } > > -int rockchip_gem_mmap_buf(struct drm_gem_object *obj, > - struct vm_area_struct *vma) > +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj, > + struct vm_area_struct *vma) > + > { > + int ret; > struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); > struct drm_device *drm = obj->dev; > - unsigned long vm_size; > > - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; > - vm_size = vma->vm_end - vma->vm_start; > - > - if (vm_size > obj->size) > - return -EINVAL; > + /* > + * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear > + * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap(). > + */ > + vma->vm_flags &= ~VM_PFNMAP; > > - return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, > + ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, > obj->size, &rk_obj->dma_attrs); > + if (ret) > + drm_gem_vm_close(vma); > + > + return ret; > } > > -/* drm driver mmap file operations */ > -int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) > +int rockchip_gem_mmap_buf(struct drm_gem_object *obj, > + struct vm_area_struct *vma) > { > - struct drm_file *priv = filp->private_data; > - struct drm_device *dev = priv->minor->dev; > - struct drm_gem_object *obj; > - struct drm_vma_offset_node *node; > + struct drm_device *drm = obj->dev; > int ret; > > - if (drm_device_is_unplugged(dev)) > - return -ENODEV; > + mutex_lock(&drm->struct_mutex); > + ret = drm_gem_mmap_obj(obj, obj->size, vma); > + mutex_unlock(&drm->struct_mutex); > + if (ret) > + return ret; > > - mutex_lock(&dev->struct_mutex); > + return rockchip_drm_gem_object_mmap(obj, vma); > +} > > - node = drm_vma_offset_exact_lookup(dev->vma_offset_manager, > - vma->vm_pgoff, > - vma_pages(vma)); > - if (!node) { > - mutex_unlock(&dev->struct_mutex); > - DRM_ERROR("failed to find vma node.\n"); > - return -EINVAL; > - } else if (!drm_vma_node_is_allowed(node, filp)) { > - mutex_unlock(&dev->struct_mutex); > - return -EACCES; > - } > +/* drm driver mmap file operations */ > +int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) > +{ > + struct drm_gem_object *obj; > + int ret; > > - obj = container_of(node, struct drm_gem_object, vma_node); > - ret = rockchip_gem_mmap_buf(obj, vma); > + ret = drm_gem_mmap(filp, vma); > + if (ret) > + return ret; > > - mutex_unlock(&dev->struct_mutex); > + obj = vma->vm_private_data; > > - return ret; > + return rockchip_drm_gem_object_mmap(obj, vma); > } > > struct rockchip_gem_object * > -- > 2.4.3.573.g4eafbef > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] drm/rockchip: use drm_gem_mmap helpers @ 2015-07-07 12:04 ` Daniel Vetter 0 siblings, 0 replies; 8+ messages in thread From: Daniel Vetter @ 2015-07-07 12:04 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jul 07, 2015 at 05:03:36PM +0800, Daniel Kurtz wrote: > Rather than (incompletely [0]) re-implementing drm_gem_mmap() and > drm_gem_mmap_obj() helpers, call them directly from the rockchip mmap > routines. > > Once the core functions return successfully, the rockchip mmap routines > can still use dma_mmap_attrs() to simply mmap the entire buffer. > > [0] Previously, we were performing the mmap() without first taking a > reference on the underlying gem buffer. This could leak ptes if the gem > object is destroyed while userspace is still holding the mapping. > > Signed-off-by: Daniel Kurtz <djkurtz@chromium.org> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: stable at vger.kernel.org Applied to topic/drm-fixes to make sure it won't get lost, but I expect rockchip maintainers to pick this one up. -Daniel > > --- > drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 67 +++++++++++++++-------------- > 1 file changed, 34 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > index eb2282c..eba5f8a 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > @@ -54,55 +54,56 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj) > &rk_obj->dma_attrs); > } > > -int rockchip_gem_mmap_buf(struct drm_gem_object *obj, > - struct vm_area_struct *vma) > +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj, > + struct vm_area_struct *vma) > + > { > + int ret; > struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); > struct drm_device *drm = obj->dev; > - unsigned long vm_size; > > - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; > - vm_size = vma->vm_end - vma->vm_start; > - > - if (vm_size > obj->size) > - return -EINVAL; > + /* > + * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear > + * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap(). > + */ > + vma->vm_flags &= ~VM_PFNMAP; > > - return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, > + ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, > obj->size, &rk_obj->dma_attrs); > + if (ret) > + drm_gem_vm_close(vma); > + > + return ret; > } > > -/* drm driver mmap file operations */ > -int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) > +int rockchip_gem_mmap_buf(struct drm_gem_object *obj, > + struct vm_area_struct *vma) > { > - struct drm_file *priv = filp->private_data; > - struct drm_device *dev = priv->minor->dev; > - struct drm_gem_object *obj; > - struct drm_vma_offset_node *node; > + struct drm_device *drm = obj->dev; > int ret; > > - if (drm_device_is_unplugged(dev)) > - return -ENODEV; > + mutex_lock(&drm->struct_mutex); > + ret = drm_gem_mmap_obj(obj, obj->size, vma); > + mutex_unlock(&drm->struct_mutex); > + if (ret) > + return ret; > > - mutex_lock(&dev->struct_mutex); > + return rockchip_drm_gem_object_mmap(obj, vma); > +} > > - node = drm_vma_offset_exact_lookup(dev->vma_offset_manager, > - vma->vm_pgoff, > - vma_pages(vma)); > - if (!node) { > - mutex_unlock(&dev->struct_mutex); > - DRM_ERROR("failed to find vma node.\n"); > - return -EINVAL; > - } else if (!drm_vma_node_is_allowed(node, filp)) { > - mutex_unlock(&dev->struct_mutex); > - return -EACCES; > - } > +/* drm driver mmap file operations */ > +int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) > +{ > + struct drm_gem_object *obj; > + int ret; > > - obj = container_of(node, struct drm_gem_object, vma_node); > - ret = rockchip_gem_mmap_buf(obj, vma); > + ret = drm_gem_mmap(filp, vma); > + if (ret) > + return ret; > > - mutex_unlock(&dev->struct_mutex); > + obj = vma->vm_private_data; > > - return ret; > + return rockchip_drm_gem_object_mmap(obj, vma); > } > > struct rockchip_gem_object * > -- > 2.4.3.573.g4eafbef > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/rockchip: use drm_gem_mmap helpers 2015-07-07 12:04 ` Daniel Vetter (?) (?) @ 2015-07-08 4:06 ` Mark yao 2015-07-08 7:35 ` Daniel Vetter -1 siblings, 1 reply; 8+ messages in thread From: Mark yao @ 2015-07-08 4:06 UTC (permalink / raw) To: Daniel Kurtz, Kees Cook, Douglas Anderson, stable, David Airlie, Heiko Stuebner, open, list, DRM DRIVERS FOR ROCKCHIP, moderated [-- Attachment #1.1: Type: text/plain, Size: 4465 bytes --] On 2015年07月07日 20:04, Daniel Vetter wrote: > On Tue, Jul 07, 2015 at 05:03:36PM +0800, Daniel Kurtz wrote: >> Rather than (incompletely [0]) re-implementing drm_gem_mmap() and >> drm_gem_mmap_obj() helpers, call them directly from the rockchip mmap >> routines. >> >> Once the core functions return successfully, the rockchip mmap routines >> can still use dma_mmap_attrs() to simply mmap the entire buffer. >> >> [0] Previously, we were performing the mmap() without first taking a >> reference on the underlying gem buffer. This could leak ptes if the gem >> object is destroyed while userspace is still holding the mapping. >> >> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org> >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> Cc: stable@vger.kernel.org > Applied to topic/drm-fixes to make sure it won't get lost, but I expect > rockchip maintainers to pick this one up. > -Daniel I try to pick this patch up, but found it conflicts with patch [0]. Can you fix it? [0]https://patchwork.kernel.org/patch/6226591/ -Mark >> --- >> drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 67 +++++++++++++++-------------- >> 1 file changed, 34 insertions(+), 33 deletions(-) >> >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c >> index eb2282c..eba5f8a 100644 >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c >> @@ -54,55 +54,56 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj) >> &rk_obj->dma_attrs); >> } >> >> -int rockchip_gem_mmap_buf(struct drm_gem_object *obj, >> - struct vm_area_struct *vma) >> +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj, >> + struct vm_area_struct *vma) >> + >> { >> + int ret; >> struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); >> struct drm_device *drm = obj->dev; >> - unsigned long vm_size; >> >> - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; >> - vm_size = vma->vm_end - vma->vm_start; >> - >> - if (vm_size > obj->size) >> - return -EINVAL; >> + /* >> + * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear >> + * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap(). >> + */ >> + vma->vm_flags &= ~VM_PFNMAP; >> >> - return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, >> + ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, >> obj->size, &rk_obj->dma_attrs); >> + if (ret) >> + drm_gem_vm_close(vma); >> + >> + return ret; >> } >> >> -/* drm driver mmap file operations */ >> -int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) >> +int rockchip_gem_mmap_buf(struct drm_gem_object *obj, >> + struct vm_area_struct *vma) >> { >> - struct drm_file *priv = filp->private_data; >> - struct drm_device *dev = priv->minor->dev; >> - struct drm_gem_object *obj; >> - struct drm_vma_offset_node *node; >> + struct drm_device *drm = obj->dev; >> int ret; >> >> - if (drm_device_is_unplugged(dev)) >> - return -ENODEV; >> + mutex_lock(&drm->struct_mutex); >> + ret = drm_gem_mmap_obj(obj, obj->size, vma); >> + mutex_unlock(&drm->struct_mutex); >> + if (ret) >> + return ret; >> >> - mutex_lock(&dev->struct_mutex); >> + return rockchip_drm_gem_object_mmap(obj, vma); >> +} >> >> - node = drm_vma_offset_exact_lookup(dev->vma_offset_manager, >> - vma->vm_pgoff, >> - vma_pages(vma)); >> - if (!node) { >> - mutex_unlock(&dev->struct_mutex); >> - DRM_ERROR("failed to find vma node.\n"); >> - return -EINVAL; >> - } else if (!drm_vma_node_is_allowed(node, filp)) { >> - mutex_unlock(&dev->struct_mutex); >> - return -EACCES; >> - } >> +/* drm driver mmap file operations */ >> +int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) >> +{ >> + struct drm_gem_object *obj; >> + int ret; >> >> - obj = container_of(node, struct drm_gem_object, vma_node); >> - ret = rockchip_gem_mmap_buf(obj, vma); >> + ret = drm_gem_mmap(filp, vma); >> + if (ret) >> + return ret; >> >> - mutex_unlock(&dev->struct_mutex); >> + obj = vma->vm_private_data; >> >> - return ret; >> + return rockchip_drm_gem_object_mmap(obj, vma); >> } >> >> struct rockchip_gem_object * >> -- >> 2.4.3.573.g4eafbef >> -- Ｍark [-- Attachment #1.2: Type: text/html, Size: 5414 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/rockchip: use drm_gem_mmap helpers 2015-07-08 4:06 ` Mark yao @ 2015-07-08 7:35 ` Daniel Vetter 0 siblings, 0 replies; 8+ messages in thread From: Daniel Vetter @ 2015-07-08 7:35 UTC (permalink / raw) To: Mark yao Cc: DRM DRIVERS FOR ROCKCHIP, list, Douglas Anderson, stable, open list, ARM/Rockchip SoC support, moderated, open, ARM/Rockchip SoC support, Kees Cook On Wed, Jul 08, 2015 at 12:06:53PM +0800, Mark yao wrote: > On 2015年07月07日 20:04, Daniel Vetter wrote: > >On Tue, Jul 07, 2015 at 05:03:36PM +0800, Daniel Kurtz wrote: > >>Rather than (incompletely [0]) re-implementing drm_gem_mmap() and > >>drm_gem_mmap_obj() helpers, call them directly from the rockchip mmap > >>routines. > >> > >>Once the core functions return successfully, the rockchip mmap routines > >>can still use dma_mmap_attrs() to simply mmap the entire buffer. > >> > >>[0] Previously, we were performing the mmap() without first taking a > >>reference on the underlying gem buffer. This could leak ptes if the gem > >>object is destroyed while userspace is still holding the mapping. > >> > >>Signed-off-by: Daniel Kurtz <djkurtz@chromium.org> > >>Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > >>Cc: stable@vger.kernel.org > >Applied to topic/drm-fixes to make sure it won't get lost, but I expect > >rockchip maintainers to pick this one up. > >-Daniel > I try to pick this patch up, but found it conflicts with patch [0]. Can you > fix it? > > [0]https://patchwork.kernel.org/patch/6226591/ Imo this should be the other way round since Daniel's patch fixes a fairly serious issue: Apply this fix first, rebase&queue the polish for -next. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-07-08 7:35 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-07-07 9:03 [PATCH] drm/rockchip: use drm_gem_mmap helpers Daniel Kurtz 2015-07-07 9:03 ` Daniel Kurtz 2015-07-07 9:03 ` Daniel Kurtz 2015-07-07 12:04 ` Daniel Vetter 2015-07-07 12:04 ` Daniel Vetter 2015-07-07 12:04 ` Daniel Vetter 2015-07-08 4:06 ` Mark yao 2015-07-08 7:35 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.