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-ldb.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/imx/imx-ldb.c') diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index 66ea68e8da87..1823af9936c9 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -590,9 +590,8 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data) int ret; int i; - imx_ldb = devm_kzalloc(dev, sizeof(*imx_ldb), GFP_KERNEL); - if (!imx_ldb) - return -ENOMEM; + imx_ldb = dev_get_drvdata(dev); + memset(imx_ldb, 0, sizeof(*imx_ldb)); imx_ldb->regmap = syscon_regmap_lookup_by_phandle(np, "gpr"); if (IS_ERR(imx_ldb->regmap)) { @@ -700,8 +699,6 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data) } } - dev_set_drvdata(dev, imx_ldb); - return 0; free_child: @@ -733,6 +730,14 @@ static const struct component_ops imx_ldb_ops = { static int imx_ldb_probe(struct platform_device *pdev) { + struct imx_ldb *imx_ldb; + + imx_ldb = devm_kzalloc(&pdev->dev, sizeof(*imx_ldb), GFP_KERNEL); + if (!imx_ldb) + return -ENOMEM; + + platform_set_drvdata(pdev, imx_ldb); + return component_add(&pdev->dev, &imx_ldb_ops); } -- cgit v1.2.3-70-g09d2 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-ldb.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-70-g09d2 From 8e91cbb820981a8a51be55499f860b1968308960 Mon Sep 17 00:00:00 2001 From: Marco Felsch Date: Thu, 21 Nov 2019 17:55:09 +0100 Subject: drm/imx: imx-ldb: remove useless enum Since commit 5e501ed7253b ("drm/imx: imx-ldb: allow to determine bus format from the connected panel") the enum isn't used anymore. Drop it to cleanup the code a bit. Signed-off-by: Marco Felsch Reviewed-by: Philipp Zabel Signed-off-by: Philipp Zabel --- drivers/gpu/drm/imx/imx-ldb.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'drivers/gpu/drm/imx/imx-ldb.c') diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index f6e05fcda379..909682a74474 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -464,11 +464,6 @@ static int imx_ldb_register(struct drm_device *drm, return 0; } -enum { - LVDS_BIT_MAP_SPWG, - LVDS_BIT_MAP_JEIDA -}; - struct imx_ldb_bit_mapping { u32 bus_format; u32 datawidth; -- cgit v1.2.3-70-g09d2 From 3b2a999582c467d1883716b37ffcc00178a13713 Mon Sep 17 00:00:00 2001 From: Liu Ying Date: Thu, 9 Jul 2020 10:28:52 +0800 Subject: drm/imx: imx-ldb: Disable both channels for split mode in enc->disable() Both of the two LVDS channels should be disabled for split mode in the encoder's ->disable() callback, because they are enabled in the encoder's ->enable() callback. Fixes: 6556f7f82b9c ("drm: imx: Move imx-drm driver out of staging") Cc: Philipp Zabel Cc: Sascha Hauer Cc: Pengutronix Kernel Team Cc: NXP Linux Team Cc: Signed-off-by: Liu Ying Signed-off-by: Philipp Zabel --- drivers/gpu/drm/imx/imx-ldb.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/imx/imx-ldb.c') diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index 909682a74474..8791d60be92e 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -296,18 +296,19 @@ static void imx_ldb_encoder_disable(struct drm_encoder *encoder) { struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder); struct imx_ldb *ldb = imx_ldb_ch->ldb; + int dual = ldb->ldb_ctrl & LDB_SPLIT_MODE_EN; int mux, ret; drm_panel_disable(imx_ldb_ch->panel); - if (imx_ldb_ch == &ldb->channel[0]) + if (imx_ldb_ch == &ldb->channel[0] || dual) ldb->ldb_ctrl &= ~LDB_CH0_MODE_EN_MASK; - else if (imx_ldb_ch == &ldb->channel[1]) + if (imx_ldb_ch == &ldb->channel[1] || dual) ldb->ldb_ctrl &= ~LDB_CH1_MODE_EN_MASK; regmap_write(ldb->regmap, IOMUXC_GPR2, ldb->ldb_ctrl); - if (ldb->ldb_ctrl & LDB_SPLIT_MODE_EN) { + if (dual) { clk_disable_unprepare(ldb->clk[0]); clk_disable_unprepare(ldb->clk[1]); } -- cgit v1.2.3-70-g09d2