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/parallel-display.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/imx/parallel-display.c') diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index ac916c84a631..622eabe9efb3 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -326,9 +326,8 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) u32 bus_format = 0; const char *fmt; - imxpd = devm_kzalloc(dev, sizeof(*imxpd), GFP_KERNEL); - if (!imxpd) - return -ENOMEM; + imxpd = dev_get_drvdata(dev); + memset(imxpd, 0, sizeof(*imxpd)); edidp = of_get_property(np, "edid", &imxpd->edid_len); if (edidp) @@ -359,8 +358,6 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) if (ret) return ret; - dev_set_drvdata(dev, imxpd); - return 0; } @@ -382,6 +379,14 @@ static const struct component_ops imx_pd_ops = { static int imx_pd_probe(struct platform_device *pdev) { + struct imx_parallel_display *imxpd; + + imxpd = devm_kzalloc(&pdev->dev, sizeof(*imxpd), GFP_KERNEL); + if (!imxpd) + return -ENOMEM; + + platform_set_drvdata(pdev, imxpd); + return component_add(&pdev->dev, &imx_pd_ops); } -- cgit v1.2.3 From dbd1d67d9201ee1eeb770a4fa4459fa76018192f Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Mon, 9 Mar 2020 21:18:33 +0100 Subject: drm/imx: parallel-display: Adjust bus_flags handling The bus_flags handling logic does not seem to cover all potential usecases. Specifically, this seems to fail with an "edt,etm0700g0edh6" display attached to an 24bit display interface, with interface-pix-fmt = "rgb24" set in DT. This patch fixes the problem by overriding the imx_crtc_state->bus_flags from the imxpd->bus_flags only if the DT property "interface-pix-fmt" is present or if the DI provides no formats. Signed-off-by: Marek Vasut Signed-off-by: Philipp Zabel --- drivers/gpu/drm/imx/parallel-display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/imx/parallel-display.c') diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index 622eabe9efb3..2d773f090603 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -217,7 +217,7 @@ static int imx_pd_bridge_atomic_check(struct drm_bridge *bridge, if (next_bridge_state) bus_flags = next_bridge_state->input_bus_cfg.flags; - else if (!imxpd->bus_format && di->num_bus_formats) + else if (di->num_bus_formats) bus_flags = di->bus_flags; else bus_flags = imxpd->bus_flags; -- 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/parallel-display.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 853fe4fc757246a2da6258f1dd249e71a1bd14fd Mon Sep 17 00:00:00 2001 From: Marco Felsch Date: Wed, 20 Nov 2019 17:54:59 +0100 Subject: drm/imx: parallel-display: move panel/bridge detection to fail early We do some string parsing and string comparison in front of drm_of_find_panel_or_bridge(). All this work is useless if the call fails. Move drm_of_find_panel_or_bridge() infront of the parsing work to fail early. Signed-off-by: Marco Felsch Reviewed-by: Philipp Zabel Signed-off-by: Philipp Zabel --- drivers/gpu/drm/imx/parallel-display.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers/gpu/drm/imx/parallel-display.c') diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index 6e55bf98b05f..a831b5bd1613 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -320,6 +320,12 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) imxpd = dev_get_drvdata(dev); memset(imxpd, 0, sizeof(*imxpd)); + /* port@1 is the output port */ + ret = drm_of_find_panel_or_bridge(np, 1, 0, &imxpd->panel, + &imxpd->next_bridge); + if (ret && ret != -ENODEV) + return ret; + edidp = of_get_property(np, "edid", &imxpd->edid_len); if (edidp) imxpd->edid = kmemdup(edidp, imxpd->edid_len, GFP_KERNEL); @@ -337,12 +343,6 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) } imxpd->bus_format = bus_format; - /* port@1 is the output port */ - ret = drm_of_find_panel_or_bridge(np, 1, 0, &imxpd->panel, - &imxpd->next_bridge); - if (ret && ret != -ENODEV) - return ret; - imxpd->dev = dev; ret = imx_pd_register(drm, imxpd); -- cgit v1.2.3