| Message ID | 20180713145514.r7ukvw23kcdwc3pv@kili.mountain |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: freescale: off by one in imx1_pinconf_group_dbg_show() | expand |
On Fri, Jul 13, 2018 at 05:55:15PM +0300, Dan Carpenter wrote: > The info->groups[] array is allocated in imx1_pinctrl_parse_dt(). It > has info->ngroups elements. Thus the > here should be >= to prevent > reading one element beyond the end of the array. > > Fixes: 30612cd90005 ("pinctrl: imx1 core driver") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by: Uwe Kleine-König <u.kleine-könig@pengutronix.de> I suggest to backport this to stable. Best regards Uwe
> -----Original Message----- > From: Dan Carpenter [mailto:dan.carpenter@oracle.com] > Sent: Friday, July 13, 2018 10:55 PM > To: A.s. Dong <aisheng.dong@nxp.com>; Markus Pargmann > <mpa@pengutronix.de> > Cc: Fabio Estevam <festevam@gmail.com>; Shawn Guo > <shawnguo@kernel.org>; Stefan Agner <stefan@agner.ch>; Pengutronix > Kernel Team <kernel@pengutronix.de>; Linus Walleij > <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; kernel- > janitors@vger.kernel.org > Subject: [PATCH] pinctrl: freescale: off by one in > imx1_pinconf_group_dbg_show() > > The info->groups[] array is allocated in imx1_pinctrl_parse_dt(). It has info- > >ngroups elements. Thus the > here should be >= to prevent reading one > element beyond the end of the array. > > Fixes: 30612cd90005 ("pinctrl: imx1 core driver") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Acked-by: Dong Aisheng <Aisheng.dong@nxp.com> BTW It seems pinctrl-imx.c has the same issue although it won't trigger real error because the second check causes the return. But the fix still applies. So would you send anther fix for pinctrl-imx as well? Regards Dong Aisheng > > diff --git a/drivers/pinctrl/freescale/pinctrl-imx1-core.c > b/drivers/pinctrl/freescale/pinctrl-imx1-core.c > index c3bdd90b1422..deb7870b3d1a 100644 > --- a/drivers/pinctrl/freescale/pinctrl-imx1-core.c > +++ b/drivers/pinctrl/freescale/pinctrl-imx1-core.c > @@ -429,7 +429,7 @@ static void imx1_pinconf_group_dbg_show(struct > pinctrl_dev *pctldev, > const char *name; > int i, ret; > > - if (group > info->ngroups) > + if (group >= info->ngroups) > return; > > seq_puts(s, "\n"); -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Copy linux-imx@nxp.com > -----Original Message----- > From: A.s. Dong > Sent: Saturday, July 14, 2018 4:13 PM > To: 'Dan Carpenter' <dan.carpenter@oracle.com>; Markus Pargmann > <mpa@pengutronix.de> > Cc: Fabio Estevam <festevam@gmail.com>; Shawn Guo > <shawnguo@kernel.org>; Stefan Agner <stefan@agner.ch>; Pengutronix > Kernel Team <kernel@pengutronix.de>; Linus Walleij > <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; kernel- > janitors@vger.kernel.org > Subject: RE: [PATCH] pinctrl: freescale: off by one in > imx1_pinconf_group_dbg_show() > > > -----Original Message----- > > From: Dan Carpenter [mailto:dan.carpenter@oracle.com] > > Sent: Friday, July 13, 2018 10:55 PM > > To: A.s. Dong <aisheng.dong@nxp.com>; Markus Pargmann > > <mpa@pengutronix.de> > > Cc: Fabio Estevam <festevam@gmail.com>; Shawn Guo > > <shawnguo@kernel.org>; Stefan Agner <stefan@agner.ch>; Pengutronix > > Kernel Team <kernel@pengutronix.de>; Linus Walleij > > <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; kernel- > > janitors@vger.kernel.org > > Subject: [PATCH] pinctrl: freescale: off by one in > > imx1_pinconf_group_dbg_show() > > > > The info->groups[] array is allocated in imx1_pinctrl_parse_dt(). It > > has info- > > >ngroups elements. Thus the > here should be >= to prevent reading > > >one > > element beyond the end of the array. > > > > Fixes: 30612cd90005 ("pinctrl: imx1 core driver") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > Acked-by: Dong Aisheng <Aisheng.dong@nxp.com> > > BTW It seems pinctrl-imx.c has the same issue although it won't trigger real > error because the second check causes the return. But the fix still applies. So > would you send anther fix for pinctrl-imx as well? > > Regards > Dong Aisheng > > > > > diff --git a/drivers/pinctrl/freescale/pinctrl-imx1-core.c > > b/drivers/pinctrl/freescale/pinctrl-imx1-core.c > > index c3bdd90b1422..deb7870b3d1a 100644 > > --- a/drivers/pinctrl/freescale/pinctrl-imx1-core.c > > +++ b/drivers/pinctrl/freescale/pinctrl-imx1-core.c > > @@ -429,7 +429,7 @@ static void imx1_pinconf_group_dbg_show(struct > > pinctrl_dev *pctldev, > > const char *name; > > int i, ret; > > > > - if (group > info->ngroups) > > + if (group >= info->ngroups) > > return; > > > > seq_puts(s, "\n"); -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jul 14, 2018 at 08:14:41AM +0000, A.s. Dong wrote: > Copy linux-imx@nxp.com > > > -----Original Message----- > > From: A.s. Dong > > Sent: Saturday, July 14, 2018 4:13 PM > > To: 'Dan Carpenter' <dan.carpenter@oracle.com>; Markus Pargmann > > <mpa@pengutronix.de> > > Cc: Fabio Estevam <festevam@gmail.com>; Shawn Guo > > <shawnguo@kernel.org>; Stefan Agner <stefan@agner.ch>; Pengutronix > > Kernel Team <kernel@pengutronix.de>; Linus Walleij > > <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; kernel- > > janitors@vger.kernel.org > > Subject: RE: [PATCH] pinctrl: freescale: off by one in > > imx1_pinconf_group_dbg_show() > > > > > -----Original Message----- > > > From: Dan Carpenter [mailto:dan.carpenter@oracle.com] > > > Sent: Friday, July 13, 2018 10:55 PM > > > To: A.s. Dong <aisheng.dong@nxp.com>; Markus Pargmann > > > <mpa@pengutronix.de> > > > Cc: Fabio Estevam <festevam@gmail.com>; Shawn Guo > > > <shawnguo@kernel.org>; Stefan Agner <stefan@agner.ch>; Pengutronix > > > Kernel Team <kernel@pengutronix.de>; Linus Walleij > > > <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; kernel- > > > janitors@vger.kernel.org > > > Subject: [PATCH] pinctrl: freescale: off by one in > > > imx1_pinconf_group_dbg_show() > > > > > > The info->groups[] array is allocated in imx1_pinctrl_parse_dt(). It > > > has info- > > > >ngroups elements. Thus the > here should be >= to prevent reading > > > >one > > > element beyond the end of the array. > > > > > > Fixes: 30612cd90005 ("pinctrl: imx1 core driver") > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > Acked-by: Dong Aisheng <Aisheng.dong@nxp.com> > > > > BTW It seems pinctrl-imx.c has the same issue although it won't trigger real > > error because the second check causes the return. But the fix still applies. So > > would you send anther fix for pinctrl-imx as well? I don't know which function you are talking about. I'm looking at drivers/pinctrl/freescale/pinctrl-imx.c but I haven't seen a bug. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Dan Carpenter [mailto:dan.carpenter@oracle.com] > Sent: Saturday, July 14, 2018 7:03 PM > To: A.s. Dong <aisheng.dong@nxp.com> > Cc: Markus Pargmann <mpa@pengutronix.de>; dl-linux-imx <linux- > imx@nxp.com>; Fabio Estevam <festevam@gmail.com>; Shawn Guo > <shawnguo@kernel.org>; Stefan Agner <stefan@agner.ch>; Pengutronix > Kernel Team <kernel@pengutronix.de>; Linus Walleij > <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; kernel- > janitors@vger.kernel.org > Subject: Re: [PATCH] pinctrl: freescale: off by one in > imx1_pinconf_group_dbg_show() > > On Sat, Jul 14, 2018 at 08:14:41AM +0000, A.s. Dong wrote: > > Copy linux-imx@nxp.com > > > > > -----Original Message----- > > > From: A.s. Dong > > > Sent: Saturday, July 14, 2018 4:13 PM > > > To: 'Dan Carpenter' <dan.carpenter@oracle.com>; Markus Pargmann > > > <mpa@pengutronix.de> > > > Cc: Fabio Estevam <festevam@gmail.com>; Shawn Guo > > > <shawnguo@kernel.org>; Stefan Agner <stefan@agner.ch>; Pengutronix > > > Kernel Team <kernel@pengutronix.de>; Linus Walleij > > > <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; kernel- > > > janitors@vger.kernel.org > > > Subject: RE: [PATCH] pinctrl: freescale: off by one in > > > imx1_pinconf_group_dbg_show() > > > > > > > -----Original Message----- > > > > From: Dan Carpenter [mailto:dan.carpenter@oracle.com] > > > > Sent: Friday, July 13, 2018 10:55 PM > > > > To: A.s. Dong <aisheng.dong@nxp.com>; Markus Pargmann > > > > <mpa@pengutronix.de> > > > > Cc: Fabio Estevam <festevam@gmail.com>; Shawn Guo > > > > <shawnguo@kernel.org>; Stefan Agner <stefan@agner.ch>; > Pengutronix > > > > Kernel Team <kernel@pengutronix.de>; Linus Walleij > > > > <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; kernel- > > > > janitors@vger.kernel.org > > > > Subject: [PATCH] pinctrl: freescale: off by one in > > > > imx1_pinconf_group_dbg_show() > > > > > > > > The info->groups[] array is allocated in imx1_pinctrl_parse_dt(). > > > > It has info- > > > > >ngroups elements. Thus the > here should be >= to prevent > > > > >reading one > > > > element beyond the end of the array. > > > > > > > > Fixes: 30612cd90005 ("pinctrl: imx1 core driver") > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > > > Acked-by: Dong Aisheng <Aisheng.dong@nxp.com> > > > > > > BTW It seems pinctrl-imx.c has the same issue although it won't > > > trigger real error because the second check causes the return. But > > > the fix still applies. So would you send anther fix for pinctrl-imx as well? > > I don't know which function you are talking about. I'm looking at > drivers/pinctrl/freescale/pinctrl-imx.c but I haven't seen a bug. > I mean the same issue you fixed in this patch. See: drivers/pinctrl/freescale/pinctrl-imx.c: imx_pinconf_group_dbg_show() Shoud it be the same fix like below? if (group >= pctldev->num_groups) return; Regards Dong Aisheng > regards, > dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 13, 2018 at 4:55 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > The info->groups[] array is allocated in imx1_pinctrl_parse_dt(). It > has info->ngroups elements. Thus the > here should be >= to prevent > reading one element beyond the end of the array. > > Fixes: 30612cd90005 ("pinctrl: imx1 core driver") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Patch applied. I am not tagging for stable as it is debug code and does not affect end users. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Mon, Jul 30, 2018 at 05:43:43PM +0200, Linus Walleij wrote: > On Fri, Jul 13, 2018 at 4:55 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > The info->groups[] array is allocated in imx1_pinctrl_parse_dt(). It > > has info->ngroups elements. Thus the > here should be >= to prevent > > reading one element beyond the end of the array. > > > > Fixes: 30612cd90005 ("pinctrl: imx1 core driver") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > Patch applied. > > I am not tagging for stable as it is debug code and does not > affect end users. Not sure this is a valid reason. Distro kernels usually enable debugfs. I'd say an out-of-bounds access that can only be triggered by root should still be fixed. I won't argue but added stable to the addressees of this mail to at least raise awareness. Best regards Uwe
On Tue, Jul 31, 2018 at 10:52 AM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > On Mon, Jul 30, 2018 at 05:43:43PM +0200, Linus Walleij wrote: > > On Fri, Jul 13, 2018 at 4:55 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > > The info->groups[] array is allocated in imx1_pinctrl_parse_dt(). It > > > has info->ngroups elements. Thus the > here should be >= to prevent > > > reading one element beyond the end of the array. > > > > > > Fixes: 30612cd90005 ("pinctrl: imx1 core driver") > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > Patch applied. > > > > I am not tagging for stable as it is debug code and does not > > affect end users. > > Not sure this is a valid reason. Distro kernels usually enable debugfs. > I'd say an out-of-bounds access that can only be triggered by root > should still be fixed. I won't argue but added stable to the addressees > of this mail to at least raise awareness. I won't argue either, I will just tag it for stable. If it matters for my friends, it matters to me, so: stable it is. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pinctrl/freescale/pinctrl-imx1-core.c b/drivers/pinctrl/freescale/pinctrl-imx1-core.c index c3bdd90b1422..deb7870b3d1a 100644 --- a/drivers/pinctrl/freescale/pinctrl-imx1-core.c +++ b/drivers/pinctrl/freescale/pinctrl-imx1-core.c @@ -429,7 +429,7 @@ static void imx1_pinconf_group_dbg_show(struct pinctrl_dev *pctldev, const char *name; int i, ret; - if (group > info->ngroups) + if (group >= info->ngroups) return; seq_puts(s, "\n");
The info->groups[] array is allocated in imx1_pinctrl_parse_dt(). It has info->ngroups elements. Thus the > here should be >= to prevent reading one element beyond the end of the array. Fixes: 30612cd90005 ("pinctrl: imx1 core driver") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html