[PATCH] pinctrl: imx: off by one in imx_pinconf_group_dbg_show()

[PATCH] pinctrl: imx: off by one in imx_pinconf_group_dbg_show()

[Dan Carpenter]

The > should really be >= here.  It's harmless because
pinctrl_generic_get_group() will return a NULL if group is invalid.

Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
Reported-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 1c6bb15579e1..b04edc22dad7 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -383,7 +383,7 @@ static void imx_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
 	const char *name;
 	int i, ret;
 
-	if (group > pctldev->num_groups)
+	if (group >= pctldev->num_groups)
 		return;
 
 	seq_puts(s, "\n");

Re: [PATCH] pinctrl: imx: off by one in imx_pinconf_group_dbg_show()

[Stefan Agner]

On 19.07.2018 10:16, Dan Carpenter wrote:
> The > should really be >= here.  It's harmless because
> pinctrl_generic_get_group() will return a NULL if group is invalid.
> 
> Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
> Reported-by: Dong Aisheng <aisheng.dong@nxp.com>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Reviewed-by: Stefan Agner <stefan@agner.ch>

> ---
> 
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c
> b/drivers/pinctrl/freescale/pinctrl-imx.c
> index 1c6bb15579e1..b04edc22dad7 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> @@ -383,7 +383,7 @@ static void imx_pinconf_group_dbg_show(struct
> pinctrl_dev *pctldev,
>  	const char *name;
>  	int i, ret;
>  
> -	if (group > pctldev->num_groups)
> +	if (group >= pctldev->num_groups)
>  		return;
>  
>  	seq_puts(s, "\n");

Re: [PATCH] pinctrl: imx: off by one in imx_pinconf_group_dbg_show()

[Linus Walleij]

On Thu, Jul 19, 2018 at 10:17 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:

> The > should really be >= here.  It's harmless because
> pinctrl_generic_get_group() will return a NULL if group is invalid.
>
> Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
> Reported-by: Dong Aisheng <aisheng.dong@nxp.com>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Patch applied.

Yours,
Linus Walleij

Re: [PATCH] pinctrl: imx: off by one in imx_pinconf_group_dbg_show()

[Uwe Kleine-König]

On Sun, Jul 29, 2018 at 11:23:36PM +0200, Linus Walleij wrote:
> On Thu, Jul 19, 2018 at 10:17 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> > The > should really be >= here.  It's harmless because
> > pinctrl_generic_get_group() will return a NULL if group is invalid.
> >
> > Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
> > Reported-by: Dong Aisheng <aisheng.dong@nxp.com>
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Patch applied.

I see that you applied the imx_pinconf_group_dbg_show patch, but not the
one specific to imx1 earlier in this thread. These are orthogonal and so
both should be applied.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] pinctrl: imx: off by one in imx_pinconf_group_dbg_show()

[A.s. Dong]

> -----Original Message-----
> From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de]
> Sent: Monday, July 30, 2018 2:42 PM
> To: Linus Walleij <linus.walleij@linaro.org>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>; A.s. Dong
> <aisheng.dong@nxp.com>; Fabio Estevam <festevam@gmail.com>; kernel-
> janitors@vger.kernel.org; Stefan Agner <stefan@agner.ch>; open list:GPIO
> SUBSYSTEM <linux-gpio@vger.kernel.org>; Sascha Hauer
> <kernel@pengutronix.de>; Shawn Guo <shawnguo@kernel.org>
> Subject: Re: [PATCH] pinctrl: imx: off by one in
> imx_pinconf_group_dbg_show()
> 
> On Sun, Jul 29, 2018 at 11:23:36PM +0200, Linus Walleij wrote:
> > On Thu, Jul 19, 2018 at 10:17 AM Dan Carpenter
> <dan.carpenter@oracle.com> wrote:
> >
> > > The > should really be >= here.  It's harmless because
> > > pinctrl_generic_get_group() will return a NULL if group is invalid.
> > >
> > > Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core
> > > driver")
> > > Reported-by: Dong Aisheng <aisheng.dong@nxp.com>
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > Patch applied.
> 
> I see that you applied the imx_pinconf_group_dbg_show patch, but not the
> one specific to imx1 earlier in this thread. These are orthogonal and so both
> should be applied.
> 

Right.

BTW just let you know, unlike that patch, this one is just a simple improvement 
and don't have to go to stable tree.

Regards
Dong Aisheng

> Best regards
> Uwe
> 
> --
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> w.pengutronix.de%2F&amp;data=02%7C01%7Caisheng.dong%40nxp.com%7
> C4d3365909e4347b199f608d5f5e787e5%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636685297127964877&amp;sdata=RpfBCPK%2Fuom9gUGN
> wPCAY%2FR8nK67S66ryT9z5g3HM1k%3D&amp;reserved=0  |
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pinctrl: imx: off by one in imx_pinconf_group_dbg_show()

[Linus Walleij]

On Mon, Jul 30, 2018 at 8:41 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Sun, Jul 29, 2018 at 11:23:36PM +0200, Linus Walleij wrote:
> > On Thu, Jul 19, 2018 at 10:17 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > > The > should really be >= here.  It's harmless because
> > > pinctrl_generic_get_group() will return a NULL if group is invalid.
> > >
> > > Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
> > > Reported-by: Dong Aisheng <aisheng.dong@nxp.com>
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > Patch applied.
>
> I see that you applied the imx_pinconf_group_dbg_show patch, but not the
> one specific to imx1 earlier in this thread. These are orthogonal and so
> both should be applied.

Oh I'm sorry if I missed something... which subject does the missing
patch have?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pinctrl: imx: off by one in imx_pinconf_group_dbg_show()

[Uwe Kleine-König]

On Mon, Jul 30, 2018 at 11:04:59AM +0200, Linus Walleij wrote:
> On Mon, Jul 30, 2018 at 8:41 AM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > On Sun, Jul 29, 2018 at 11:23:36PM +0200, Linus Walleij wrote:
> > > On Thu, Jul 19, 2018 at 10:17 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > >
> > > > The > should really be >= here.  It's harmless because
> > > > pinctrl_generic_get_group() will return a NULL if group is invalid.
> > > >
> > > > Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
> > > > Reported-by: Dong Aisheng <aisheng.dong@nxp.com>
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > >
> > > Patch applied.
> >
> > I see that you applied the imx_pinconf_group_dbg_show patch, but not the
> > one specific to imx1 earlier in this thread. These are orthogonal and so
> > both should be applied.
> 
> Oh I'm sorry if I missed something... which subject does the missing
> patch have?

The subject is:

pinctrl: freescale: off by one in imx1_pinconf_group_dbg_show() 

(Note it only differs by s/imx/imx1/ from the patch you already took.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pinctrl: imx: off by one in imx_pinconf_group_dbg_show()

[Uwe Kleine-König]

On Mon, Jul 30, 2018 at 08:47:15AM +0000, A.s. Dong wrote:
> > -----Original Message-----
> > From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de]
> > Sent: Monday, July 30, 2018 2:42 PM
> > To: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Dan Carpenter <dan.carpenter@oracle.com>; A.s. Dong
> > <aisheng.dong@nxp.com>; Fabio Estevam <festevam@gmail.com>; kernel-
> > janitors@vger.kernel.org; Stefan Agner <stefan@agner.ch>; open list:GPIO
> > SUBSYSTEM <linux-gpio@vger.kernel.org>; Sascha Hauer
> > <kernel@pengutronix.de>; Shawn Guo <shawnguo@kernel.org>
> > Subject: Re: [PATCH] pinctrl: imx: off by one in
> > imx_pinconf_group_dbg_show()
> > 
> > On Sun, Jul 29, 2018 at 11:23:36PM +0200, Linus Walleij wrote:
> > > On Thu, Jul 19, 2018 at 10:17 AM Dan Carpenter
> > <dan.carpenter@oracle.com> wrote:
> > >
> > > > The > should really be >= here.  It's harmless because
> > > > pinctrl_generic_get_group() will return a NULL if group is invalid.
> > > >
> > > > Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core
> > > > driver")
> > > > Reported-by: Dong Aisheng <aisheng.dong@nxp.com>
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > >
> > > Patch applied.
> > 
> > I see that you applied the imx_pinconf_group_dbg_show patch, but not the
> > one specific to imx1 earlier in this thread. These are orthogonal and so both
> > should be applied.
> > 
> 
> Right.
> 
> BTW just let you know, unlike that patch, this one is just a simple improvement 
> and don't have to go to stable tree.

To clearify: the imx1 change (ie. the one currently missed) is the
critical one as it fixes an out-of-bounds access.
imx_pinconf_group_dbg_show was harmless, the only change is that with it
applied there is one "\n" less in the output.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html