Thread (11 messages) flat view 11 messages, 3 authors, 2021-03-24

Re: [PATCH] [v2] drm/imx: imx-ldb: fix out of bounds array access warning

From: Arnd Bergmann <arnd@kernel.org>
Date: 2021-03-24 16:43:50
Also in: dri-devel, lkml

On Wed, Mar 24, 2021 at 3:20 PM Joe Perches [off-list ref] wrote:
On Wed, 2021-03-24 at 13:17 +0100, Arnd Bergmann wrote:
quoted
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>
---
v2: fix subject line
    expand patch description
    print mux number
    check upper bound as well
[]
quoted
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
[]
quoted
@@ -197,6 +197,12 @@ 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__, ERR_PTR(mux));
This does not compile without warnings.

drivers/gpu/drm/imx/imx-ldb.c: In function ‘imx_ldb_encoder_enable’:
drivers/gpu/drm/imx/imx-ldb.c:201:22: warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘void *’ [-Wformat=]
  201 |   dev_warn(ldb->dev, "%s: invalid mux %d\n",
      |                      ^~~~~~~~~~~~~~~~~~~~~~

If you want to use ERR_PTR, the %d should be %pe as ERR_PTR
is converting an int a void * to decode the error type and
emit it as a string.
Sorry about that.

I decided against using ERR_PTR() in order to also check for
positive array overflow, but the version I tested was different from
the version I sent.

v3 coming.

         Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help