linux-kernel - [PATCH] Revert "drm/imx: don't destroy mode objects manually on driver unbind"

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-next>] [day] [month] [year] [list]

Message-Id: <20181016160923.2042-1-stefan@agner.ch>
Date:   Tue, 16 Oct 2018 18:09:23 +0200
From:   Stefan Agner <stefan@...er.ch>
To:     p.zabel@...gutronix.de
Cc:     airlied@...ux.ie, rmk+kernel@...linux.org.uk,
        l.stach@...gutronix.de, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, Stefan Agner <stefan@...er.ch>
Subject: [PATCH] Revert "drm/imx: don't destroy mode objects manually on driver unbind"

This reverts commit 8e3b16e2117409625b89807de3912ff773aea354.

Using the component framework requires all components to undo in
->unbind what ->bind does. Unfortunately that particular commit
broke this rule. In particular, this is an issue if a single
component during probe fails. In that case, component_bind_all()
calls unbind on already succussfully bound components, and then
frees memory allocated using devm. If one of those components
registered e.g. an encoder with the framework then this leads to
use after free situations.

Revert the commit to ensure that all components properly undo
what ->bind does.

Link: https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg233327.html
Suggested-by: Russell King <rmk+kernel@...linux.org.uk>
Signed-off-by: Stefan Agner <stefan@...er.ch>
---
 drivers/gpu/drm/imx/imx-drm-core.c     | 4 ++--
 drivers/gpu/drm/imx/imx-ldb.c          | 6 ++++++
 drivers/gpu/drm/imx/imx-tve.c          | 3 +++
 drivers/gpu/drm/imx/parallel-display.c | 3 +++
 4 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 5ea0c82f9957..caa6061a98ba 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -305,11 +305,11 @@ static void imx_drm_unbind(struct device *dev)
 
 	drm_fb_cma_fbdev_fini(drm);
 
-	drm_mode_config_cleanup(drm);
-
 	component_unbind_all(drm->dev, drm);
 	dev_set_drvdata(dev, NULL);
 
+	drm_mode_config_cleanup(drm);
+
 	drm_dev_put(drm);
 }
 
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index 3bd0f8a18e74..592aabc4a262 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -723,6 +723,12 @@ static void imx_ldb_unbind(struct device *dev, struct device *master,
 		if (channel->panel)
 			drm_panel_detach(channel->panel);
 
+		if (!channel->connector.funcs)
+			continue;
+
+		channel->connector.funcs->destroy(&channel->connector);
+		channel->encoder.funcs->destroy(&channel->encoder);
+
 		kfree(channel->edid);
 		i2c_put_adapter(channel->ddc);
 	}
diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index cffd3310240e..8d6e89ce1edb 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -673,6 +673,9 @@ static void imx_tve_unbind(struct device *dev, struct device *master,
 {
 	struct imx_tve *tve = dev_get_drvdata(dev);
 
+	tve->connector.funcs->destroy(&tve->connector);
+	tve->encoder.funcs->destroy(&tve->encoder);
+
 	if (!IS_ERR(tve->dac_reg))
 		regulator_disable(tve->dac_reg);
 }
diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
index aefd04e18f93..6f11bffcde37 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -258,6 +258,9 @@ static void imx_pd_unbind(struct device *dev, struct device *master,
 	if (imxpd->panel)
 		drm_panel_detach(imxpd->panel);
 
+	imxpd->encoder.funcs->destroy(&imxpd->encoder);
+	imxpd->connector.funcs->destroy(&imxpd->connector);
+
 	kfree(imxpd->edid);
 }
 
-- 
2.19.1