From ba807c94f67fd64b3051199810d9e4dd209fdc00 Mon Sep 17 00:00:00 2001 From: Philipp Zabel Date: Thu, 11 Jun 2020 14:43:31 +0200 Subject: drm/imx: fix use after free Component driver structures allocated with devm_kmalloc() in bind() are freed automatically after unbind(). Since the contained drm structures are accessed afterwards in drm_mode_config_cleanup(), move the allocation into probe() to extend the driver structure's lifetime to the lifetime of the device. This should eventually be changed to use drm resource managed allocations with lifetime of the drm device. We also need to ensure that all componets are available during the unbind() so we need to call component_unbind_all() before we free non-devres resources like planes. Note this patch fixes the the use after free bug but introduces a possible boot loop issue. The issue is triggered if the HDMI support is enabled and a component driver always return -EPROBE_DEFER, see discussion [1] for more details. [1] https://lkml.org/lkml/2020/3/24/1467 Fixes: 17b5001b5143 ("imx-drm: convert to componentised device support") Signed-off-by: Philipp Zabel [m.felsch@pengutronix: fix imx_tve_probe()] [m.felsch@pengutronix: resort component_unbind_all()) [m.felsch@pengutronix: adapt commit message] Signed-off-by: Marco Felsch Signed-off-by: Philipp Zabel --- drivers/gpu/drm/imx/imx-tve.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/imx/imx-tve.c') diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c index ee63782c77e9..82d1ee1fb0c8 100644 --- a/drivers/gpu/drm/imx/imx-tve.c +++ b/drivers/gpu/drm/imx/imx-tve.c @@ -542,9 +542,8 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) int irq; int ret; - tve = devm_kzalloc(dev, sizeof(*tve), GFP_KERNEL); - if (!tve) - return -ENOMEM; + tve = dev_get_drvdata(dev); + memset(tve, 0, sizeof(*tve)); tve->dev = dev; spin_lock_init(&tve->lock); @@ -655,8 +654,6 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) if (ret) return ret; - dev_set_drvdata(dev, tve); - return 0; } @@ -676,6 +673,14 @@ static const struct component_ops imx_tve_ops = { static int imx_tve_probe(struct platform_device *pdev) { + struct imx_tve *tve; + + tve = devm_kzalloc(&pdev->dev, sizeof(*tve), GFP_KERNEL); + if (!tve) + return -ENOMEM; + + platform_set_drvdata(pdev, tve); + return component_add(&pdev->dev, &imx_tve_ops); } -- cgit v1.2.3 From 7bb58b987fee26da2a1665c01033022624986b7c Mon Sep 17 00:00:00 2001 From: Marco Felsch Date: Thu, 11 Jun 2020 14:43:32 +0200 Subject: drm/imx: tve: fix regulator_disable error path Add missing regulator_disable() as devm_action to avoid dedicated unbind() callback and fix the missing error handling. Fixes: fcbc51e54d2a ("staging: drm/imx: Add support for Television Encoder (TVEv2)") Signed-off-by: Marco Felsch Signed-off-by: Philipp Zabel --- drivers/gpu/drm/imx/imx-tve.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'drivers/gpu/drm/imx/imx-tve.c') diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c index 82d1ee1fb0c8..3758de3e09bd 100644 --- a/drivers/gpu/drm/imx/imx-tve.c +++ b/drivers/gpu/drm/imx/imx-tve.c @@ -490,6 +490,13 @@ static int imx_tve_register(struct drm_device *drm, struct imx_tve *tve) return 0; } +static void imx_tve_disable_regulator(void *data) +{ + struct imx_tve *tve = data; + + regulator_disable(tve->dac_reg); +} + static bool imx_tve_readable_reg(struct device *dev, unsigned int reg) { return (reg % 4 == 0) && (reg <= 0xdc); @@ -613,6 +620,9 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) ret = regulator_enable(tve->dac_reg); if (ret) return ret; + ret = devm_add_action_or_reset(dev, imx_tve_disable_regulator, tve); + if (ret) + return ret; } tve->clk = devm_clk_get(dev, "tve"); @@ -657,18 +667,8 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) return 0; } -static void imx_tve_unbind(struct device *dev, struct device *master, - void *data) -{ - struct imx_tve *tve = dev_get_drvdata(dev); - - if (!IS_ERR(tve->dac_reg)) - regulator_disable(tve->dac_reg); -} - static const struct component_ops imx_tve_ops = { .bind = imx_tve_bind, - .unbind = imx_tve_unbind, }; static int imx_tve_probe(struct platform_device *pdev) -- cgit v1.2.3 From 816df9447ec2bca5ff56ee157f4f706a7a614300 Mon Sep 17 00:00:00 2001 From: Marco Felsch Date: Thu, 21 Nov 2019 14:47:43 +0100 Subject: drm/imx: drop useless best_encoder callback The best_encoder() callback is used by the drm-core to find an encoder if the connector is connected to multiple encoders but the parallel, tve and ldb uses always the 1-encoder : 1-connector setup. Such a simple setup can be handled by the drm-core. Signed-off-by: Marco Felsch Reviewed-by: Philipp Zabel Signed-off-by: Philipp Zabel --- drivers/gpu/drm/imx/imx-ldb.c | 9 --------- drivers/gpu/drm/imx/imx-tve.c | 9 --------- drivers/gpu/drm/imx/parallel-display.c | 9 --------- 3 files changed, 27 deletions(-) (limited to 'drivers/gpu/drm/imx/imx-tve.c') diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index 1823af9936c9..f6e05fcda379 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -156,14 +156,6 @@ static int imx_ldb_connector_get_modes(struct drm_connector *connector) return num_modes; } -static struct drm_encoder *imx_ldb_connector_best_encoder( - struct drm_connector *connector) -{ - struct imx_ldb_channel *imx_ldb_ch = con_to_imx_ldb_ch(connector); - - return &imx_ldb_ch->encoder; -} - static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno, unsigned long serial_clk, unsigned long di_clk) { @@ -391,7 +383,6 @@ static const struct drm_connector_funcs imx_ldb_connector_funcs = { static const struct drm_connector_helper_funcs imx_ldb_connector_helper_funcs = { .get_modes = imx_ldb_connector_get_modes, - .best_encoder = imx_ldb_connector_best_encoder, }; static const struct drm_encoder_helper_funcs imx_ldb_encoder_helper_funcs = { diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c index 3758de3e09bd..c5025307a9a7 100644 --- a/drivers/gpu/drm/imx/imx-tve.c +++ b/drivers/gpu/drm/imx/imx-tve.c @@ -260,14 +260,6 @@ static int imx_tve_connector_mode_valid(struct drm_connector *connector, return MODE_BAD; } -static struct drm_encoder *imx_tve_connector_best_encoder( - struct drm_connector *connector) -{ - struct imx_tve *tve = con_to_tve(connector); - - return &tve->encoder; -} - static void imx_tve_encoder_mode_set(struct drm_encoder *encoder, struct drm_display_mode *orig_mode, struct drm_display_mode *mode) @@ -345,7 +337,6 @@ static const struct drm_connector_funcs imx_tve_connector_funcs = { static const struct drm_connector_helper_funcs imx_tve_connector_helper_funcs = { .get_modes = imx_tve_connector_get_modes, - .best_encoder = imx_tve_connector_best_encoder, .mode_valid = imx_tve_connector_mode_valid, }; diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index 2d773f090603..6e55bf98b05f 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -88,14 +88,6 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector) return num_modes; } -static struct drm_encoder *imx_pd_connector_best_encoder( - struct drm_connector *connector) -{ - struct imx_parallel_display *imxpd = con_to_imxpd(connector); - - return &imxpd->encoder; -} - static void imx_pd_bridge_enable(struct drm_bridge *bridge) { struct imx_parallel_display *imxpd = bridge_to_imxpd(bridge); @@ -254,7 +246,6 @@ static const struct drm_connector_funcs imx_pd_connector_funcs = { static const struct drm_connector_helper_funcs imx_pd_connector_helper_funcs = { .get_modes = imx_pd_connector_get_modes, - .best_encoder = imx_pd_connector_best_encoder, }; static const struct drm_bridge_funcs imx_pd_bridge_funcs = { -- cgit v1.2.3 From 408a85e31e3e5127c91e082c3544082ef1ba48d3 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sun, 5 Apr 2020 11:01:49 +0200 Subject: drm/imx: imx-tve: Delete an error message in imx_tve_bind() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function “platform_get_irq” can log an error already. Thus omit a redundant message for the exception handling in the calling function. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Philipp Zabel --- drivers/gpu/drm/imx/imx-tve.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/gpu/drm/imx/imx-tve.c') diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c index c5025307a9a7..813bb6156a68 100644 --- a/drivers/gpu/drm/imx/imx-tve.c +++ b/drivers/gpu/drm/imx/imx-tve.c @@ -591,10 +591,8 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) } irq = platform_get_irq(pdev, 0); - if (irq < 0) { - dev_err(dev, "failed to get irq\n"); + if (irq < 0) return irq; - } ret = devm_request_threaded_irq(dev, irq, NULL, imx_tve_irq_handler, IRQF_ONESHOT, -- cgit v1.2.3