diff mbox

drm: Undo damage to page_flip_ioctl

Message ID 20161003082827.11586-1-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Oct. 3, 2016, 8:28 a.m. UTC 
I screwed up rebasing of my patch in

commit 43968d7b806d7a7e021261294c583a216fddf0e5
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed Sep 21 10:59:24 2016 +0200

    drm: Extract drm_plane.[hc]

which meant on error paths drm_crtc_vblank_put could be called without
a get, leading to an underrun of the refcount.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98020
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Michel Dänzer <michel@daenzer.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_plane.c | 81 ++++++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 42 deletions(-)

Comments

Michel Dänzer Oct. 3, 2016, 8:35 a.m. UTC | #1 
On 03/10/16 05:28 PM, Daniel Vetter wrote:
> I screwed up rebasing of my patch in
> 
> commit 43968d7b806d7a7e021261294c583a216fddf0e5
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Wed Sep 21 10:59:24 2016 +0200
> 
>     drm: Extract drm_plane.[hc]
> 
> which meant on error paths drm_crtc_vblank_put could be called without
> a get, leading to an underrun of the refcount.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98020
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Michel Dänzer <michel@daenzer.net>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Daniel Vetter Oct. 3, 2016, 1:01 p.m. UTC | #2 
On Mon, Oct 03, 2016 at 05:35:05PM +0900, Michel Dänzer wrote:
> On 03/10/16 05:28 PM, Daniel Vetter wrote:
> > I screwed up rebasing of my patch in
> > 
> > commit 43968d7b806d7a7e021261294c583a216fddf0e5
> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Date:   Wed Sep 21 10:59:24 2016 +0200
> > 
> >     drm: Extract drm_plane.[hc]
> > 
> > which meant on error paths drm_crtc_vblank_put could be called without
> > a get, leading to an underrun of the refcount.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98020
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Cc: Michel Dänzer <michel@daenzer.net>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>

Applied to drm-misc, with the t-b from the bug report. Will send out the
pull request asap.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 783aef8acab7..249c0ae52c6d 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -783,6 +783,45 @@  int drm_mode_page_flip_ioctl(struct drm_device *dev,
 	if (!crtc)
 		return -ENOENT;
 
+	if (crtc->funcs->page_flip_target) {
+		u32 current_vblank;
+		int r;
+
+		r = drm_crtc_vblank_get(crtc);
+		if (r)
+			return r;
+
+		current_vblank = drm_crtc_vblank_count(crtc);
+
+		switch (page_flip->flags & DRM_MODE_PAGE_FLIP_TARGET) {
+		case DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE:
+			if ((int)(target_vblank - current_vblank) > 1) {
+				DRM_DEBUG("Invalid absolute flip target %u, "
+					  "must be <= %u\n", target_vblank,
+					  current_vblank + 1);
+				drm_crtc_vblank_put(crtc);
+				return -EINVAL;
+			}
+			break;
+		case DRM_MODE_PAGE_FLIP_TARGET_RELATIVE:
+			if (target_vblank != 0 && target_vblank != 1) {
+				DRM_DEBUG("Invalid relative flip target %u, "
+					  "must be 0 or 1\n", target_vblank);
+				drm_crtc_vblank_put(crtc);
+				return -EINVAL;
+			}
+			target_vblank += current_vblank;
+			break;
+		default:
+			target_vblank = current_vblank +
+				!(page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC);
+			break;
+		}
+	} else if (crtc->funcs->page_flip == NULL ||
+		   (page_flip->flags & DRM_MODE_PAGE_FLIP_TARGET)) {
+		return -EINVAL;
+	}
+
 	drm_modeset_lock_crtc(crtc, crtc->primary);
 	if (crtc->primary->fb == NULL) {
 		/* The framebuffer is currently unbound, presumably
@@ -793,9 +832,6 @@  int drm_mode_page_flip_ioctl(struct drm_device *dev,
 		goto out;
 	}
 
-	if (crtc->funcs->page_flip == NULL)
-		goto out;
-
 	fb = drm_framebuffer_lookup(dev, page_flip->fb_id);
 	if (!fb) {
 		ret = -ENOENT;
@@ -839,45 +875,6 @@  int drm_mode_page_flip_ioctl(struct drm_device *dev,
 	}
 
 	crtc->primary->old_fb = crtc->primary->fb;
-	if (crtc->funcs->page_flip_target) {
-		u32 current_vblank;
-		int r;
-
-		r = drm_crtc_vblank_get(crtc);
-		if (r)
-			return r;
-
-		current_vblank = drm_crtc_vblank_count(crtc);
-
-		switch (page_flip->flags & DRM_MODE_PAGE_FLIP_TARGET) {
-		case DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE:
-			if ((int)(target_vblank - current_vblank) > 1) {
-				DRM_DEBUG("Invalid absolute flip target %u, "
-					  "must be <= %u\n", target_vblank,
-					  current_vblank + 1);
-				drm_crtc_vblank_put(crtc);
-				return -EINVAL;
-			}
-			break;
-		case DRM_MODE_PAGE_FLIP_TARGET_RELATIVE:
-			if (target_vblank != 0 && target_vblank != 1) {
-				DRM_DEBUG("Invalid relative flip target %u, "
-					  "must be 0 or 1\n", target_vblank);
-				drm_crtc_vblank_put(crtc);
-				return -EINVAL;
-			}
-			target_vblank += current_vblank;
-			break;
-		default:
-			target_vblank = current_vblank +
-				!(page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC);
-			break;
-		}
-	} else if (crtc->funcs->page_flip == NULL ||
-		   (page_flip->flags & DRM_MODE_PAGE_FLIP_TARGET)) {
-		return -EINVAL;
-	}
-
 	if (crtc->funcs->page_flip_target)
 		ret = crtc->funcs->page_flip_target(crtc, fb, e,
 						    page_flip->flags,