| Message ID | 20210324164750.3833773-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] drm/imx: imx-ldb: fix out of bounds array access warning | expand |
On Wed, 2021-03-24 at 17:47 +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > 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. 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. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Liu Ying <victor.liu@nxp.com> Thanks, Liu Ying > --- > 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(+) > > diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c > index dbfe39e2f7f6..565482e2b816 100644 > --- a/drivers/gpu/drm/imx/imx-ldb.c > +++ b/drivers/gpu/drm/imx/imx-ldb.c > @@ -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__);
On Thu, Mar 25, 2021 at 10:03:23AM +0800, Liu Ying wrote: > On Wed, 2021-03-24 at 17:47 +0100, Arnd Bergmann wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > > > 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. > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Reviewed-by: Liu Ying <victor.liu@nxp.com> Thank you, applied to imx-drm/fixes. regards Philipp
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index dbfe39e2f7f6..565482e2b816 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -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__);