linux-kernel - [PATCH 3.16 230/328] 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: <lsq.1544392233.730715302@decadent.org.uk>
Date:   Sun, 09 Dec 2018 21:50:33 +0000
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org,
        "Daniel Vetter" <daniel.vetter@...ll.ch>,
        "Sean Paul" <seanpaul@...omium.org>,
        "Emil Lundmark" <lndmrk@...omium.org>
Subject: [PATCH 3.16 230/328] drm: udl: Destroy framebuffer only if it was
 initialized

3.16.62-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Emil Lundmark <lndmrk@...omium.org>

commit fcb74da1eb8edd3a4ef9b9724f88ed709d684227 upstream.

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>
Reviewed-by: Sean Paul <seanpaul@...omium.org>
Acked-by: Daniel Vetter <daniel.vetter@...ll.ch>
Signed-off-by: Emil Lundmark <lndmrk@...omium.org>
Signed-off-by: Sean Paul <seanpaul@...omium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20180528142711.142466-1-lndmrk@chromium.org
Signed-off-by: Sean Paul <seanpaul@...omium.org>
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 drivers/gpu/drm/udl/udl_fb.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -574,9 +574,11 @@ static void udl_fbdev_destroy(struct drm
 		framebuffer_release(info);
 	}
 	drm_fb_helper_fini(&ufbdev->helper);
-	drm_framebuffer_unregister_private(&ufbdev->ufb.base);
-	drm_framebuffer_cleanup(&ufbdev->ufb.base);
-	drm_gem_object_unreference_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_unreference_unlocked(&ufbdev->ufb.obj->base);
+	}
 }

 int udl_fbdev_init(struct drm_device *dev)