Blob Blame History Raw
From cd88ee3d68f305c27018820adaaf3050b6e134b1 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Fri, 20 Jul 2018 12:48:02 +0200
Subject: drm/cirrus: Use drm_framebuffer_put to avoid kernel oops in clean-up
References: bsc#1101822
Git-commit: abf7b30d7f61d981bfcca65d1e8331b27021b475
Patch-mainline: v4.20-rc1

In the Cirrus driver, the regular clean-up code also performs the clean-up
of a failed initialization. If the fbdev's framebuffer was not initialized,
the clean-up will fail within drm_framebuffer_unregister_private. Booting
with cirrus.bpp=16 triggers this bug.

The framebuffer is currently stored directly within struct cirrus_fbdev. To
fix the bug, we turn it into a pointer that is only set for initialized
framebuffers. The fbdev's clean-up code skips uninitialized framebuffers.

The memory for struct drm_framebuffer is allocated dynamically. This requires
additional error handling within cirrusfb_create. The framebuffer clean-up is
now performed by drm_framebuffer_put, which also frees the data strcuture's
memory.

Link: https://bugzilla.suse.com/show_bug.cgi?id=1101822
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/cirrus/cirrus_drv.h   |    2 -
 drivers/gpu/drm/cirrus/cirrus_fbdev.c |   49 ++++++++++++++++++++--------------
 drivers/gpu/drm/cirrus/cirrus_mode.c  |    2 -
 3 files changed, 32 insertions(+), 21 deletions(-)

--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
@@ -152,7 +152,7 @@ struct cirrus_device {
 
 struct cirrus_fbdev {
 	struct drm_fb_helper helper;
-	struct cirrus_framebuffer gfb;
+	struct cirrus_framebuffer *gfb;
 	void *sysram;
 	int size;
 	int x1, y1, x2, y2; /* dirty rect */
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -22,14 +22,14 @@ static void cirrus_dirty_update(struct c
 	struct drm_gem_object *obj;
 	struct cirrus_bo *bo;
 	int src_offset, dst_offset;
-	int bpp = afbdev->gfb.base.format->cpp[0];
+	int bpp = afbdev->gfb->base.format->cpp[0];
 	int ret = -EBUSY;
 	bool unmap = false;
 	bool store_for_later = false;
 	int x2, y2;
 	unsigned long flags;
 
-	obj = afbdev->gfb.obj;
+	obj = afbdev->gfb->obj;
 	bo = gem_to_cirrus_bo(obj);
 
 	/*
@@ -82,7 +82,7 @@ static void cirrus_dirty_update(struct c
 	}
 	for (i = y; i < y + height; i++) {
 		/* assume equal stride for now */
-		src_offset = dst_offset = i * afbdev->gfb.base.pitches[0] + (x * bpp);
+		src_offset = dst_offset = i * afbdev->gfb->base.pitches[0] + (x * bpp);
 		memcpy_toio(bo->kmap.virtual + src_offset, afbdev->sysram + src_offset, width * bpp);
 
 	}
@@ -165,7 +165,7 @@ static int cirrusfb_create(struct drm_fb
 		container_of(helper, struct cirrus_fbdev, helper);
 	struct cirrus_device *cdev = gfbdev->helper.dev->dev_private;
 	struct fb_info *info;
-	struct drm_framebuffer *fb;
+	struct cirrus_framebuffer *fb;
 	struct drm_mode_fb_cmd2 mode_cmd;
 	void *sysram;
 	struct drm_gem_object *gobj = NULL;
@@ -192,33 +192,36 @@ static int cirrusfb_create(struct drm_fb
 		return -ENOMEM;
 
 	info = drm_fb_helper_alloc_fbi(helper);
-	if (IS_ERR(info))
-		return PTR_ERR(info);
+	if (IS_ERR(info)) {
+		ret = PTR_ERR(info);
+		goto err_vfree;
+	}
 
 	info->par = gfbdev;
 
-	ret = cirrus_framebuffer_init(cdev->dev, &gfbdev->gfb, &mode_cmd, gobj);
+	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
+	if (!fb) {
+		ret = -ENOMEM;
+		goto err_drm_gem_object_put_unlocked;
+	}
+
+	ret = cirrus_framebuffer_init(cdev->dev, fb, &mode_cmd, gobj);
 	if (ret)
-		return ret;
+		goto err_kfree;
 
 	gfbdev->sysram = sysram;
 	gfbdev->size = size;
-
-	fb = &gfbdev->gfb.base;
-	if (!fb) {
-		DRM_INFO("fb is NULL\n");
-		return -EINVAL;
-	}
+	gfbdev->gfb = fb;
 
 	/* setup helper */
-	gfbdev->helper.fb = fb;
+	gfbdev->helper.fb = &fb->base;
 
 	strcpy(info->fix.id, "cirrusdrmfb");
 
 	info->flags = FBINFO_DEFAULT;
 	info->fbops = &cirrusfb_ops;
 
-	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
+	drm_fb_helper_fill_fix(info, fb->base.pitches[0], fb->base.format->depth);
 	drm_fb_helper_fill_var(info, &gfbdev->helper, sizes->fb_width,
 			       sizes->fb_height);
 
@@ -238,16 +241,24 @@ static int cirrusfb_create(struct drm_fb
 	DRM_INFO("fb mappable at 0x%lX\n", info->fix.smem_start);
 	DRM_INFO("vram aper at 0x%lX\n", (unsigned long)info->fix.smem_start);
 	DRM_INFO("size %lu\n", (unsigned long)info->fix.smem_len);
-	DRM_INFO("fb depth is %d\n", fb->format->depth);
-	DRM_INFO("   pitch is %d\n", fb->pitches[0]);
+	DRM_INFO("fb depth is %d\n", fb->base.format->depth);
+	DRM_INFO("   pitch is %d\n", fb->base.pitches[0]);
 
 	return 0;
+
+err_kfree:
+	kfree(fb);
+err_drm_gem_object_put_unlocked:
+	drm_gem_object_put_unlocked(gobj);
+err_vfree:
+	vfree(sysram);
+	return ret;
 }
 
 static int cirrus_fbdev_destroy(struct drm_device *dev,
 				struct cirrus_fbdev *gfbdev)
 {
-	struct cirrus_framebuffer *gfb = &gfbdev->gfb;
+	struct cirrus_framebuffer *gfb = gfbdev->gfb;
 
 	drm_fb_helper_unregister_fbi(&gfbdev->helper);
 
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -133,7 +133,7 @@ static int cirrus_crtc_do_set_base(struc
 		return ret;
 	}
 
-	if (&cdev->mode_info.gfbdev->gfb == cirrus_fb) {
+	if (cdev->mode_info.gfbdev->gfb == cirrus_fb) {
 		/* if pushing console in kmap it */
 		ret = ttm_bo_kmap(&bo->bo, 0, bo->bo.num_pages, &bo->kmap);
 		if (ret)