[PATCH] drm/mgag200: don't pass NULL file_priv to drm_gem_object_lookup()

[PATCH] drm/mgag200: don't pass NULL file_priv to drm_gem_object_lookup()

[Jan Beulich]

Commit bf89209a6d ("drm/mga200g: Hold a proper reference for
cursor_set") clearly didn't take the call site in
drm_fb_helper.c:restore_fbdev_mode() into account, which passes NULL 
for file_priv and hence causes drm_gem_object_lookup() to fault. Move
the lookup back to before "obj" is actually needed, adjusting error
paths suitably once again.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

--- 4.3-rc6/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ 4.3-rc6-mgag200-cursor-set/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -40,7 +40,7 @@ int mga_crtc_cursor_set(struct drm_crtc
 	struct mgag200_bo *pixels_2 = mdev->cursor.pixels_2;
 	struct mgag200_bo *pixels_current = mdev->cursor.pixels_current;
 	struct mgag200_bo *pixels_prev = mdev->cursor.pixels_prev;
-	struct drm_gem_object *obj;
+	struct drm_gem_object *obj = NULL;
 	struct mgag200_bo *bo = NULL;
 	int ret = 0;
 	unsigned int i, row, col;
@@ -70,15 +70,11 @@ int mga_crtc_cursor_set(struct drm_crtc
 	BUG_ON(pixels_2 != pixels_current && pixels_2 != pixels_prev);
 	BUG_ON(pixels_current == pixels_prev);
 
-	obj = drm_gem_object_lookup(dev, file_priv, handle);
-	if (!obj)
-		return -ENOENT;
-
 	ret = mgag200_bo_reserve(pixels_1, true);
 	if (ret) {
 		WREG8(MGA_CURPOSXL, 0);
 		WREG8(MGA_CURPOSXH, 0);
-		goto out_unref;
+		return ret;
 	}
 	ret = mgag200_bo_reserve(pixels_2, true);
 	if (ret) {
@@ -110,6 +106,12 @@ int mga_crtc_cursor_set(struct drm_crtc
 		}
 	}
 
+	obj = drm_gem_object_lookup(dev, file_priv, handle);
+	if (!obj) {
+		ret = -ENOENT;
+		goto out1;
+	}
+
 	bo = gem_to_mga_bo(obj);
 	ret = mgag200_bo_reserve(bo, true);
 	if (ret) {
@@ -243,13 +245,13 @@ int mga_crtc_cursor_set(struct drm_crtc
  out2:
 	mgag200_bo_unreserve(bo);
  out1:
+	if (obj)
+		drm_gem_object_unreference_unlocked(obj);
 	if (ret)
 		mga_hide_cursor(mdev);
 	mgag200_bo_unreserve(pixels_1);
 out_unreserve1:
 	mgag200_bo_unreserve(pixels_2);
-out_unref:
-	drm_gem_object_unreference_unlocked(obj);
 
 	return ret;
 }



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

Re: [PATCH] drm/mgag200: don't pass NULL file_priv to drm_gem_object_lookup()

[Daniel Vetter]

On Mon, Oct 19, 2015 at 04:27:18AM -0600, Jan Beulich wrote:
> Commit bf89209a6d ("drm/mga200g: Hold a proper reference for
> cursor_set") clearly didn't take the call site in
> drm_fb_helper.c:restore_fbdev_mode() into account, which passes NULL 
> for file_priv and hence causes drm_gem_object_lookup() to fault. Move
> the lookup back to before "obj" is actually needed, adjusting error
> paths suitably once again.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thierry Reding <treding@nvidia.com>

Oops, totally forgot that handle=0 is used to disable the cursor. Thanks
for the fix.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Dave, since this is in 4.3 can you pls pick this up for drm-fixes?

Thanks, Daniel

> ---
>  drivers/gpu/drm/mgag200/mgag200_cursor.c |   18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> --- 4.3-rc6/drivers/gpu/drm/mgag200/mgag200_cursor.c
> +++ 4.3-rc6-mgag200-cursor-set/drivers/gpu/drm/mgag200/mgag200_cursor.c
> @@ -40,7 +40,7 @@ int mga_crtc_cursor_set(struct drm_crtc
>  	struct mgag200_bo *pixels_2 = mdev->cursor.pixels_2;
>  	struct mgag200_bo *pixels_current = mdev->cursor.pixels_current;
>  	struct mgag200_bo *pixels_prev = mdev->cursor.pixels_prev;
> -	struct drm_gem_object *obj;
> +	struct drm_gem_object *obj = NULL;
>  	struct mgag200_bo *bo = NULL;
>  	int ret = 0;
>  	unsigned int i, row, col;
> @@ -70,15 +70,11 @@ int mga_crtc_cursor_set(struct drm_crtc
>  	BUG_ON(pixels_2 != pixels_current && pixels_2 != pixels_prev);
>  	BUG_ON(pixels_current == pixels_prev);
>  
> -	obj = drm_gem_object_lookup(dev, file_priv, handle);
> -	if (!obj)
> -		return -ENOENT;
> -
>  	ret = mgag200_bo_reserve(pixels_1, true);
>  	if (ret) {
>  		WREG8(MGA_CURPOSXL, 0);
>  		WREG8(MGA_CURPOSXH, 0);
> -		goto out_unref;
> +		return ret;
>  	}
>  	ret = mgag200_bo_reserve(pixels_2, true);
>  	if (ret) {
> @@ -110,6 +106,12 @@ int mga_crtc_cursor_set(struct drm_crtc
>  		}
>  	}
>  
> +	obj = drm_gem_object_lookup(dev, file_priv, handle);
> +	if (!obj) {
> +		ret = -ENOENT;
> +		goto out1;
> +	}
> +
>  	bo = gem_to_mga_bo(obj);
>  	ret = mgag200_bo_reserve(bo, true);
>  	if (ret) {
> @@ -243,13 +245,13 @@ int mga_crtc_cursor_set(struct drm_crtc
>   out2:
>  	mgag200_bo_unreserve(bo);
>   out1:
> +	if (obj)
> +		drm_gem_object_unreference_unlocked(obj);
>  	if (ret)
>  		mga_hide_cursor(mdev);
>  	mgag200_bo_unreserve(pixels_1);
>  out_unreserve1:
>  	mgag200_bo_unreserve(pixels_2);
> -out_unref:
> -	drm_gem_object_unreference_unlocked(obj);
>  
>  	return ret;
>  }
> 
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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