* [PATCH] drm/imx: fix out of bounds array access warning
@ 2021-03-23 13:05 Arnd Bergmann
2021-03-23 14:02 ` Fabio Estevam
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Arnd Bergmann @ 2021-03-23 13:05 UTC (permalink / raw)
To: Philipp Zabel, David Airlie, Daniel Vetter, Shawn Guo, Sascha Hauer
Cc: Arnd Bergmann, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Marco Felsch, Joe Perches, Laurent Pinchart,
Liu Ying, Thomas Zimmermann, dri-devel, linux-arm-kernel,
linux-kernel
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.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/gpu/drm/imx/imx-ldb.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index dbfe39e2f7f6..1210360cec8a 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -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) {
+ dev_warn(ldb->dev,
+ "%s: invalid mux\n", __func__);
+ return;
+ }
+
drm_panel_prepare(imx_ldb_ch->panel);
if (dual) {
@@ -255,6 +261,12 @@ 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) {
+ dev_warn(ldb->dev,
+ "%s: invalid mux\n", __func__);
+ return;
+ }
+
if (mode->clock > 170000) {
dev_warn(ldb->dev,
"%s: mode exceeds 170 MHz pixel clock\n", __func__);
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/imx: fix out of bounds array access warning
2021-03-23 13:05 [PATCH] drm/imx: fix out of bounds array access warning Arnd Bergmann
@ 2021-03-23 14:02 ` Fabio Estevam
2021-03-24 3:03 ` Liu Ying
2021-03-23 14:19 ` Joe Perches
2021-03-24 3:30 ` Liu Ying
2 siblings, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2021-03-23 14:02 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Philipp Zabel, David Airlie, Daniel Vetter, Shawn Guo,
Sascha Hauer, Arnd Bergmann, Pengutronix Kernel Team,
NXP Linux Team, Marco Felsch, Joe Perches, Laurent Pinchart,
Liu Ying, Thomas Zimmermann, DRI mailing list,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-kernel
Hi Arnd,
On Tue, Mar 23, 2021 at 10:05 AM Arnd Bergmann <arnd@kernel.org> 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]
What about making the driver depend on OF instead (like it is done in
DRM_IMX_HDMI) ?
--- a/drivers/gpu/drm/imx/Kconfig
+++ b/drivers/gpu/drm/imx/Kconfig
@@ -27,7 +27,7 @@ config DRM_IMX_TVE
config DRM_IMX_LDB
tristate "Support for LVDS displays"
- depends on DRM_IMX && MFD_SYSCON
+ depends on DRM_IMX && MFD_SYSCON && OF
depends on COMMON_CLK
select DRM_PANEL
help
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/imx: fix out of bounds array access warning
2021-03-23 13:05 [PATCH] drm/imx: fix out of bounds array access warning Arnd Bergmann
2021-03-23 14:02 ` Fabio Estevam
@ 2021-03-23 14:19 ` Joe Perches
2021-03-24 2:50 ` Liu Ying
2021-03-24 3:30 ` Liu Ying
2 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2021-03-23 14:19 UTC (permalink / raw)
To: Arnd Bergmann, Philipp Zabel, David Airlie, Daniel Vetter,
Shawn Guo, Sascha Hauer
Cc: Arnd Bergmann, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Marco Felsch, Laurent Pinchart, Liu Ying,
Thomas Zimmermann, dri-devel, linux-arm-kernel, linux-kernel
On Tue, 2021-03-23 at 14:05 +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.
[]
> diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
[]
> @@ -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) {
> + dev_warn(ldb->dev,
> + "%s: invalid mux\n", __func__);
trivia:
Any real reason to make this 2 lines? It fits nicely in 80 chars. Maybe:
dev_warn(ldb->dev, "%s: invalid mux: %d\n", __func__, mux);
or maybe:
dev_warn(ldb->dev, "%s: invalid mux: %pe\n",
__func__, ERR_PTR(mux));
> @@ -255,6 +261,12 @@ imx_ldb_encoder_atomic_mode_set(struct drm_encoder *encoder,
[]
> + if (mux < 0) {
> + dev_warn(ldb->dev,
> + "%s: invalid mux\n", __func__);
etc...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/imx: fix out of bounds array access warning
2021-03-23 14:19 ` Joe Perches
@ 2021-03-24 2:50 ` Liu Ying
0 siblings, 0 replies; 6+ messages in thread
From: Liu Ying @ 2021-03-24 2:50 UTC (permalink / raw)
To: Joe Perches, Arnd Bergmann, Philipp Zabel, David Airlie,
Daniel Vetter, Shawn Guo, Sascha Hauer
Cc: Arnd Bergmann, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Marco Felsch, Laurent Pinchart,
Thomas Zimmermann, dri-devel, linux-arm-kernel, linux-kernel
On Tue, 2021-03-23 at 07:19 -0700, Joe Perches wrote:
> On Tue, 2021-03-23 at 14:05 +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.
> []
> > diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
> []
> > @@ -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) {
> > + dev_warn(ldb->dev,
> > + "%s: invalid mux\n", __func__);
>
> trivia:
>
> Any real reason to make this 2 lines? It fits nicely in 80 chars. Maybe:
>
> dev_warn(ldb->dev, "%s: invalid mux: %d\n", __func__, mux);
>
> or maybe:
>
> dev_warn(ldb->dev, "%s: invalid mux: %pe\n",
> __func__, ERR_PTR(mux));
+1
The second one looks better as it's more informative.
Regards,
Liu Ying
>
> > @@ -255,6 +261,12 @@ imx_ldb_encoder_atomic_mode_set(struct drm_encoder *encoder,
> []
> > + if (mux < 0) {
> > + dev_warn(ldb->dev,
> > + "%s: invalid mux\n", __func__);
>
> etc...
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/imx: fix out of bounds array access warning
2021-03-23 14:02 ` Fabio Estevam
@ 2021-03-24 3:03 ` Liu Ying
0 siblings, 0 replies; 6+ messages in thread
From: Liu Ying @ 2021-03-24 3:03 UTC (permalink / raw)
To: Fabio Estevam, Arnd Bergmann
Cc: Philipp Zabel, David Airlie, Daniel Vetter, Shawn Guo,
Sascha Hauer, Arnd Bergmann, Pengutronix Kernel Team,
NXP Linux Team, Marco Felsch, Joe Perches, Laurent Pinchart,
Thomas Zimmermann, DRI mailing list,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-kernel
Hi Fabio,
On Tue, 2021-03-23 at 11:02 -0300, Fabio Estevam wrote:
> Hi Arnd,
>
> On Tue, Mar 23, 2021 at 10:05 AM Arnd Bergmann <arnd@kernel.org> 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]
>
> What about making the driver depend on OF instead (like it is done in
> DRM_IMX_HDMI) ?
The below patch made DRM_IMX_HDMI depend on OF,
because of_drm_find_bridge() is not defined when OF is disabled.
drm/imx: dw_hdmi-imx: depend on OF to fix randconfig compile tests on
x86_64
It doesn't look like DRM_IMX_LDB is in the same case.
Moreover, even if OF is enabled, drm_of_encoder_active_endpoint() is
likely to return -EINVAL. So, it looks ok to add an error check.
Regards,
Liu Ying
>
> --- a/drivers/gpu/drm/imx/Kconfig
> +++ b/drivers/gpu/drm/imx/Kconfig
> @@ -27,7 +27,7 @@ config DRM_IMX_TVE
>
> config DRM_IMX_LDB
> tristate "Support for LVDS displays"
> - depends on DRM_IMX && MFD_SYSCON
> + depends on DRM_IMX && MFD_SYSCON && OF
> depends on COMMON_CLK
> select DRM_PANEL
> help
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/imx: fix out of bounds array access warning
2021-03-23 13:05 [PATCH] drm/imx: fix out of bounds array access warning Arnd Bergmann
2021-03-23 14:02 ` Fabio Estevam
2021-03-23 14:19 ` Joe Perches
@ 2021-03-24 3:30 ` Liu Ying
2 siblings, 0 replies; 6+ messages in thread
From: Liu Ying @ 2021-03-24 3:30 UTC (permalink / raw)
To: Arnd Bergmann, Philipp Zabel, David Airlie, Daniel Vetter,
Shawn Guo, Sascha Hauer
Cc: Arnd Bergmann, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Marco Felsch, Joe Perches, Laurent Pinchart,
Thomas Zimmermann, dri-devel, linux-arm-kernel, linux-kernel
Hi Arnd,
Thanks for your patch.
It would be good to improve the patch's head line to something like:
drm/imx: imx-ldb: fix out of bounds array access warning
Regards,
Liu Ying
On Tue, 2021-03-23 at 14:05 +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.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/gpu/drm/imx/imx-ldb.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
> index dbfe39e2f7f6..1210360cec8a 100644
> --- a/drivers/gpu/drm/imx/imx-ldb.c
> +++ b/drivers/gpu/drm/imx/imx-ldb.c
> @@ -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) {
> + dev_warn(ldb->dev,
> + "%s: invalid mux\n", __func__);
> + return;
> + }
> +
> drm_panel_prepare(imx_ldb_ch->panel);
>
> if (dual) {
> @@ -255,6 +261,12 @@ 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) {
> + dev_warn(ldb->dev,
> + "%s: invalid mux\n", __func__);
> + return;
> + }
> +
> if (mode->clock > 170000) {
> dev_warn(ldb->dev,
> "%s: mode exceeds 170 MHz pixel clock\n", __func__);
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-24 3:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 13:05 [PATCH] drm/imx: fix out of bounds array access warning Arnd Bergmann
2021-03-23 14:02 ` Fabio Estevam
2021-03-24 3:03 ` Liu Ying
2021-03-23 14:19 ` Joe Perches
2021-03-24 2:50 ` Liu Ying
2021-03-24 3:30 ` Liu Ying
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).