Skip to contentSkip to conversation

drm/imx: imx-ldb: fix out of bounds array access warning

PATCH Applied Arnd Bergmann 3 participants 3 messages Mar 24, 2021 – Mar 25, 2021

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 drm/imx: imx-ldb: fix out of bounds array access warning

From: Arnd Bergmann <[email protected]>

When CONFIG_OF is disabled, building with 'make W=1' produces warnings about out of bounds array access:

drivers/gpu/drm/imx/imx-ldb.c: In function 'imx_ldb_set_clock.constprop':
drivers/gpu/drm/imx/imx-ldb.c:186:8: error: array subscript -22 is below array bounds of 'struct clk *[4]' [-Werror=array-bounds]

Add an error check before the index is used, which helps with the warning, as well as any possible other error condition that may be triggered at runtime.

The warning could be fixed by adding a Kconfig depedency on CONFIG_OF, but Liu Ying points out that the driver may hit the out-of-bounds problem at runtime anyway.

---
v3: fix build regression from v2
v2: fix subject line
expand patch description
print mux number
check upper bound as well

---
 drivers/gpu/drm/imx/imx-ldb.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/gpu/drm/imx/imx-ldb.c +10 -0
index dbfe39e2f7f6..565482e2b816 100644
@@ -197,6 +197,11 @@ static void imx_ldb_encoder_enable(struct drm_encoder *encoder)
 	int dual = ldb->ldb_ctrl & LDB_SPLIT_MODE_EN;
 	int mux = drm_of_encoder_active_port_id(imx_ldb_ch->child, encoder);
 
+	if (mux < 0 || mux >= ARRAY_SIZE(ldb->clk_sel)) {
+		dev_warn(ldb->dev, "%s: invalid mux %d\n", __func__, mux);
+		return;
+	}
+
 	drm_panel_prepare(imx_ldb_ch->panel);
 
 	if (dual) {
@@ -255,6 +260,11 @@ imx_ldb_encoder_atomic_mode_set(struct drm_encoder *encoder,
 	int mux = drm_of_encoder_active_port_id(imx_ldb_ch->child, encoder);
 	u32 bus_format = imx_ldb_ch->bus_format;
 
+	if (mux < 0 || mux >= ARRAY_SIZE(ldb->clk_sel)) {
+		dev_warn(ldb->dev, "%s: invalid mux %d\n", __func__, mux);
+		return;
+	}
+
 	if (mode->clock > 170000) {
 		dev_warn(ldb->dev,
 			 "%s: mode exceeds 170 MHz pixel clock\n", __func__);
Signature · 6 lines
2.29.2

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Reviewed-by on v3 1/1
#1 lore raw
Quoted 8 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

When CONFIG_OF is disabled, building with 'make W=1' produces warnings about out of bounds array access:

drivers/gpu/drm/imx/imx-ldb.c: In function 'imx_ldb_set_clock.constprop':
drivers/gpu/drm/imx/imx-ldb.c:186:8: error: array subscript -22 is below array bounds of 'struct clk *[4]' [-Werror=array-bounds]

Quoted 7 lines

Add an error check before the index is used, which helps with the warning, as well as any possible other error condition that may be triggered at runtime.

The warning could be fixed by adding a Kconfig depedency on CONFIG_OF, but Liu Ying points out that the driver may hit the out-of-bounds problem at runtime anyway.

Almost impossible to hit the out-of-bounds problem at runtime, unless something wrong happens and makes unexpected parameters(node and/or encoder) be handed over to drm_of_encoder_active_port_id(). Anyway, an error check on return value from drm_of_encoder_active_port_id() looks ok to me.

Quoted 2 lines

Thanks,
Liu Ying

Quoted 28 lines

---
v3: fix build regression from v2
v2: fix subject line
expand patch description
print mux number
check upper bound as well

---
 drivers/gpu/drm/imx/imx-ldb.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/gpu/drm/imx/imx-ldb.c +5 -0
index dbfe39e2f7f6..565482e2b816 100644
@@ -197,6 +197,11 @@ static void imx_ldb_encoder_enable(struct drm_encoder *encoder)
 	int dual = ldb->ldb_ctrl & LDB_SPLIT_MODE_EN;
 	int mux = drm_of_encoder_active_port_id(imx_ldb_ch->child, encoder);
 
+	if (mux < 0 || mux >= ARRAY_SIZE(ldb->clk_sel)) {
+		dev_warn(ldb->dev, "%s: invalid mux %d\n", __func__, mux);
+		return;
+	}
+
 	drm_panel_prepare(imx_ldb_ch->panel);
 
 	if (dual) {
@@ -255,6 +260,11 @@ imx_ldb_encoder_atomic_mode_set(struct drm_encoder *encoder,
 	int mux = drm_of_encoder_active_port_id(imx_ldb_ch->child, encoder);

drivers/gpu/drm/imx/imx-ldb.c imx_ldb_encoder_atomic_mode_set(struct drm_encoder *encoder, · line 270

Quoted 10 lines
drivers/gpu/drm/imx/imx-ldb.c +5 -0
 	u32 bus_format = imx_ldb_ch->bus_format;
 
+	if (mux < 0 || mux >= ARRAY_SIZE(ldb->clk_sel)) {
+		dev_warn(ldb->dev, "%s: invalid mux %d\n", __func__, mux);
+		return;
+	}
+
 	if (mode->clock > 170000) {
 		dev_warn(ldb->dev,
 			 "%s: mode exceeds 170 MHz pixel clock\n", __func__);

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

#2 lore raw
Quoted 13 lines · Liu Ying
Quoted 12 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

When CONFIG_OF is disabled, building with 'make W=1' produces warnings about out of bounds array access:

drivers/gpu/drm/imx/imx-ldb.c: In function 'imx_ldb_set_clock.constprop':
drivers/gpu/drm/imx/imx-ldb.c:186:8: error: array subscript -22 is below array bounds of 'struct clk *[4]' [-Werror=array-bounds]

Add an error check before the index is used, which helps with the warning, as well as any possible other error condition that may be triggered at runtime.

Quoted 6 lines
Quoted 5 lines

The warning could be fixed by adding a Kconfig depedency on CONFIG_OF, but Liu Ying points out that the driver may hit the out-of-bounds problem at runtime anyway.

Thank you, applied to imx-drm/fixes.

regards
Philipp
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel