From patchwork Fri Jul 20 11:27:43 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Zimmermann X-Patchwork-Id: 10536495 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 703396029B for ; Fri, 20 Jul 2018 11:27:58 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 57EF729191 for ; Fri, 20 Jul 2018 11:27:58 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4C0CF29200; Fri, 20 Jul 2018 11:27:58 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id D791529191 for ; Fri, 20 Jul 2018 11:27:57 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 040EA6EFAD; Fri, 20 Jul 2018 11:27:57 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6879E6EFAD for ; Fri, 20 Jul 2018 11:27:55 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 0112EACBC; Fri, 20 Jul 2018 11:27:54 +0000 (UTC) From: Thomas Zimmermann To: airlied@redhat.com, kraxel@redhat.com, virtualization@lists.linux-foundation.org, dri-devel@lists.freedesktop.org Subject: [PATCH] drm/cirrus: Use drm_framebuffer_put to avoid kernel oops in clean-up Date: Fri, 20 Jul 2018 13:27:43 +0200 Message-Id: <20180720112743.27159-1-tzimmermann@suse.de> X-Mailer: git-send-email 2.18.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Thomas Zimmermann MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 --- drivers/gpu/drm/cirrus/cirrus_drv.h | 2 +- drivers/gpu/drm/cirrus/cirrus_fbdev.c | 48 +++++++++++++++------------ drivers/gpu/drm/cirrus/cirrus_mode.c | 2 +- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h b/drivers/gpu/drm/cirrus/cirrus_drv.h index ce9db7aab225..a29f87e98d9d 100644 --- a/drivers/gpu/drm/cirrus/cirrus_drv.h +++ b/drivers/gpu/drm/cirrus/cirrus_drv.h @@ -146,7 +146,7 @@ struct cirrus_device { struct cirrus_fbdev { struct drm_fb_helper helper; - struct drm_framebuffer gfb; + struct drm_framebuffer *gfb; void *sysram; int size; int x1, y1, x2, y2; /* dirty rect */ diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c index b643ac92801c..82cc82e0bd80 100644 --- 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 cirrus_fbdev *afbdev, struct drm_gem_object *obj; struct cirrus_bo *bo; int src_offset, dst_offset; - int bpp = afbdev->gfb.format->cpp[0]; + int bpp = afbdev->gfb->format->cpp[0]; int ret = -EBUSY; bool unmap = false; bool store_for_later = false; int x2, y2; unsigned long flags; - obj = afbdev->gfb.obj[0]; + obj = afbdev->gfb->obj[0]; bo = gem_to_cirrus_bo(obj); /* @@ -82,7 +82,7 @@ static void cirrus_dirty_update(struct cirrus_fbdev *afbdev, } for (i = y; i < y + height; i++) { /* assume equal stride for now */ - src_offset = dst_offset = i * afbdev->gfb.pitches[0] + (x * bpp); + src_offset = dst_offset = i * afbdev->gfb->pitches[0] + (x * bpp); memcpy_toio(bo->kmap.virtual + src_offset, afbdev->sysram + src_offset, width * bpp); } @@ -192,23 +192,26 @@ static int cirrusfb_create(struct drm_fb_helper *helper, 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; - if (!fb) { - DRM_INFO("fb is NULL\n"); - return -EINVAL; - } + gfbdev->gfb = fb; /* setup helper */ gfbdev->helper.fb = fb; @@ -241,24 +244,27 @@ static int cirrusfb_create(struct drm_fb_helper *helper, DRM_INFO(" pitch is %d\n", fb->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 drm_framebuffer *gfb = &gfbdev->gfb; + struct drm_framebuffer *gfb = gfbdev->gfb; drm_fb_helper_unregister_fbi(&gfbdev->helper); - if (gfb->obj[0]) { - drm_gem_object_put_unlocked(gfb->obj[0]); - gfb->obj[0] = NULL; - } - vfree(gfbdev->sysram); drm_fb_helper_fini(&gfbdev->helper); - drm_framebuffer_unregister_private(gfb); - drm_framebuffer_cleanup(gfb); + if (gfb) + drm_framebuffer_put(gfb); return 0; } diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c index 336bfda40125..90a4e641d3fb 100644 --- a/drivers/gpu/drm/cirrus/cirrus_mode.c +++ b/drivers/gpu/drm/cirrus/cirrus_mode.c @@ -127,7 +127,7 @@ static int cirrus_crtc_do_set_base(struct drm_crtc *crtc, return ret; } - if (&cdev->mode_info.gfbdev->gfb == crtc->primary->fb) { + if (cdev->mode_info.gfbdev->gfb == crtc->primary->fb) { /* if pushing console in kmap it */ ret = ttm_bo_kmap(&bo->bo, 0, bo->bo.num_pages, &bo->kmap); if (ret)