diff mbox

drm/gem: Always initialize the gem object in object_init

Message ID 1390202514-32184-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Jan. 20, 2014, 7:21 a.m. UTC
At least drm/i915 expects that the obj->dev pointer is set even in
failure paths. Specifically when the shmem initialization fails we
call i915_gem_object_free which needs to deref obj->base.dev to get at
the slab pointer in the device private structure. And the shmem
allocation can easily fail when userspace is hitting open file limits.

Doing the structure init even when the shmem file allocation fails
prevents this Oops.

Testcase: igt/gem_fd_exhaustion
Reported-and-Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
References: http://lists.freedesktop.org/archives/intel-gfx/2014-January/038433.html
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_gem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

David Herrmann Jan. 20, 2014, 8:34 a.m. UTC | #1
Hi

On Mon, Jan 20, 2014 at 8:21 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> At least drm/i915 expects that the obj->dev pointer is set even in
> failure paths. Specifically when the shmem initialization fails we
> call i915_gem_object_free which needs to deref obj->base.dev to get at
> the slab pointer in the device private structure. And the shmem
> allocation can easily fail when userspace is hitting open file limits.
>
> Doing the structure init even when the shmem file allocation fails
> prevents this Oops.

Makes the error-paths simpler, and drm_gem_private_object_init()
doesn't allocate anything, so looks good:
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

> Testcase: igt/gem_fd_exhaustion
> Reported-and-Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> References: http://lists.freedesktop.org/archives/intel-gfx/2014-January/038433.html
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_gem.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 3ff39bb0402d..31c919903cd0 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -129,11 +129,12 @@ int drm_gem_object_init(struct drm_device *dev,
>  {
>         struct file *filp;
>
> +       drm_gem_private_object_init(dev, obj, size);
> +
>         filp = shmem_file_setup("drm mm object", size, VM_NORESERVE);
>         if (IS_ERR(filp))
>                 return PTR_ERR(filp);
>
> -       drm_gem_private_object_init(dev, obj, size);
>         obj->filp = filp;
>
>         return 0;
> --
> 1.8.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Chris Wilson Jan. 20, 2014, 9:11 a.m. UTC | #2
On Mon, Jan 20, 2014 at 08:21:54AM +0100, Daniel Vetter wrote:
> At least drm/i915 expects that the obj->dev pointer is set even in
> failure paths. Specifically when the shmem initialization fails we
> call i915_gem_object_free which needs to deref obj->base.dev to get at
> the slab pointer in the device private structure. And the shmem
> allocation can easily fail when userspace is hitting open file limits.
> 
> Doing the structure init even when the shmem file allocation fails
> prevents this Oops.

Regression from 
commit 89c8233f82d9c8af5b20e72e4a185a38a7d3c50b
Author: David Herrmann <dh.herrmann@gmail.com>
Date:   Thu Jul 11 11:56:32 2013 +0200

    drm/gem: simplify object initialization

> Testcase: igt/gem_fd_exhaustion
> Reported-and-Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> References: http://lists.freedesktop.org/archives/intel-gfx/2014-January/038433.html
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: David Herrmann <dh.herrmann@gmail.com>
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 3ff39bb0402d..31c919903cd0 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -129,11 +129,12 @@  int drm_gem_object_init(struct drm_device *dev,
 {
 	struct file *filp;
 
+	drm_gem_private_object_init(dev, obj, size);
+
 	filp = shmem_file_setup("drm mm object", size, VM_NORESERVE);
 	if (IS_ERR(filp))
 		return PTR_ERR(filp);
 
-	drm_gem_private_object_init(dev, obj, size);
 	obj->filp = filp;
 
 	return 0;