| Message ID | 5624E1A602000078000AC488@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
--- 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; }
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(-)