linux-kernel - [PATCH v2] drm: udl: Destroy framebuffer only if it was initialized

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20180528142711.142466-1-lndmrk@chromium.org>
Date:   Mon, 28 May 2018 16:27:11 +0200
From:   Emil Lundmark <lndmrk@...omium.org>
To:     dri-devel@...ts.freedesktop.org
Cc:     Dave Airlie <airlied@...hat.com>,
        Sean Paul <seanpaul@...omium.org>,
        linux-kernel@...r.kernel.org, Emil Lundmark <lndmrk@...omium.org>,
        Daniel Vetter <daniel@...ll.ch>,
        Stéphane Marchesin 
        <stephane.marchesin@...il.com>
Subject: [PATCH v2] drm: udl: Destroy framebuffer only if it was initialized

This fixes a NULL pointer dereference that can happen if the UDL
driver is unloaded before the framebuffer is initialized. This can
happen e.g. if the USB device is unplugged right after it was plugged
in.

As explained by Stéphane Marchesin:

It happens when fbdev is disabled (which is the case for Chrome OS).
Even though intialization of the fbdev part is optional (it's done in
udlfb_create which is the callback for fb_probe()), the teardown isn't
optional (udl_driver_unload -> udl_fbdev_cleanup ->
udl_fbdev_destroy).

Note that udl_fbdev_cleanup *tries* to be conditional (you can see it
does if (!udl->fbdev)) but that doesn't work, because udl->fbdev is
always set during udl_fbdev_init.

Suggested-by: Sean Paul <seanpaul@...omium.org>
Signed-off-by: Emil Lundmark <lndmrk@...omium.org>
---
Changes in v2:
- Updated commit message with explanation from Stéphane Marchesin

 drivers/gpu/drm/udl/udl_fb.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index 2ebdc6d5a76e..5754e37f741b 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -426,9 +426,11 @@ static void udl_fbdev_destroy(struct drm_device *dev,
 {
 	drm_fb_helper_unregister_fbi(&ufbdev->helper);
 	drm_fb_helper_fini(&ufbdev->helper);
-	drm_framebuffer_unregister_private(&ufbdev->ufb.base);
-	drm_framebuffer_cleanup(&ufbdev->ufb.base);
-	drm_gem_object_put_unlocked(&ufbdev->ufb.obj->base);
+	if (ufbdev->ufb.obj) {
+		drm_framebuffer_unregister_private(&ufbdev->ufb.base);
+		drm_framebuffer_cleanup(&ufbdev->ufb.base);
+		drm_gem_object_put_unlocked(&ufbdev->ufb.obj->base);
+	}
 }

 int udl_fbdev_init(struct drm_device *dev)
-- 
2.17.0.921.gf22659ad46-goog