* [PATCH] gpiolib: check for overflow when reading the 'ngpios' property
@ 2022-03-06 19:34 Bartosz Golaszewski
2022-03-06 22:19 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Bartosz Golaszewski @ 2022-03-06 19:34 UTC (permalink / raw)
To: Linus Walleij, Andy Shevchenko
Cc: linux-gpio, linux-kernel, Bartosz Golaszewski
The ngpio fields both in struct gpio_device as well as gpio_chip are
16-bit unsigned integers. Let's not risk an overflow and check if the
property value represented as a 32-bit unsigned integer is not greater
than U16_MAX.
Fixes: 9dbd1ab20509 ("gpiolib: check the 'ngpios' property in core gpiolib code")
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
drivers/gpio/gpiolib.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a3d14277f17c..3c4f47b9ab57 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -677,6 +677,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
else if (ret)
goto err_free_descs;
+ if (ngpios > U16_MAX) {
+ ret = EINVAL;
+ goto err_free_descs;
+ }
+
gc->ngpio = ngpios;
}
--
2.30.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] gpiolib: check for overflow when reading the 'ngpios' property
2022-03-06 19:34 [PATCH] gpiolib: check for overflow when reading the 'ngpios' property Bartosz Golaszewski
@ 2022-03-06 22:19 ` Andy Shevchenko
2022-03-06 22:23 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2022-03-06 22:19 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Andy Shevchenko, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List
On Mon, Mar 7, 2022 at 12:11 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> The ngpio fields both in struct gpio_device as well as gpio_chip are
> 16-bit unsigned integers. Let's not risk an overflow and check if the
> property value represented as a 32-bit unsigned integer is not greater
> than U16_MAX.
...
> + if (ngpios > U16_MAX) {
> + ret = EINVAL;
> + goto err_free_descs;
> + }
I don't think it's a fatal error in this case. I would perhaps print a
warning and simply use a masked (which is done implicitly by an
assignment of the different type) value. Note, the above is buggy on
the buggy DTs, where the upper part of the value is not used. After
this patch you effectively make a regression on, yes, broken DTs.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gpiolib: check for overflow when reading the 'ngpios' property
2022-03-06 22:19 ` Andy Shevchenko
@ 2022-03-06 22:23 ` Andy Shevchenko
2022-03-07 9:53 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2022-03-06 22:23 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Andy Shevchenko, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List
On Mon, Mar 7, 2022 at 12:19 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Mar 7, 2022 at 12:11 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > The ngpio fields both in struct gpio_device as well as gpio_chip are
> > 16-bit unsigned integers. Let's not risk an overflow and check if the
> > property value represented as a 32-bit unsigned integer is not greater
> > than U16_MAX.
>
> ...
>
> > + if (ngpios > U16_MAX) {
> > + ret = EINVAL;
> > + goto err_free_descs;
> > + }
>
> I don't think it's a fatal error in this case. I would perhaps print a
> warning and simply use a masked (which is done implicitly by an
> assignment of the different type) value. Note, the above is buggy on
> the buggy DTs, where the upper part of the value is not used. After
> this patch you effectively make a regression on, yes, broken DTs.
Like
if (ngpios > U16_MAX)
chip_warn(gc, "line cnt %u is greater than supported; use
%u\n", ngpios, (u16)ngpio);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gpiolib: check for overflow when reading the 'ngpios' property
2022-03-06 22:23 ` Andy Shevchenko
@ 2022-03-07 9:53 ` Andy Shevchenko
2022-03-07 10:08 ` Bartosz Golaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2022-03-07 9:53 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List
On Mon, Mar 07, 2022 at 12:23:03AM +0200, Andy Shevchenko wrote:
> On Mon, Mar 7, 2022 at 12:19 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Mar 7, 2022 at 12:11 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > The ngpio fields both in struct gpio_device as well as gpio_chip are
> > > 16-bit unsigned integers. Let's not risk an overflow and check if the
> > > property value represented as a 32-bit unsigned integer is not greater
> > > than U16_MAX.
> >
> > ...
> >
> > > + if (ngpios > U16_MAX) {
> > > + ret = EINVAL;
> > > + goto err_free_descs;
> > > + }
> >
> > I don't think it's a fatal error in this case. I would perhaps print a
> > warning and simply use a masked (which is done implicitly by an
> > assignment of the different type) value. Note, the above is buggy on
> > the buggy DTs, where the upper part of the value is not used. After
> > this patch you effectively make a regression on, yes, broken DTs.
>
> Like
>
> if (ngpios > U16_MAX)
> chip_warn(gc, "line cnt %u is greater than supported; use
> %u\n", ngpios, (u16)ngpio);
Or to be on safer side move this after == 0 check as
if (gc->ngpio != ngpios)
chip_warn(gc, "line cnt %u is greater than supported; use %u\n", ngpios, gc->ngpio);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gpiolib: check for overflow when reading the 'ngpios' property
2022-03-07 9:53 ` Andy Shevchenko
@ 2022-03-07 10:08 ` Bartosz Golaszewski
2022-03-07 10:23 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Bartosz Golaszewski @ 2022-03-07 10:08 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List
On Mon, Mar 7, 2022 at 10:53 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Mar 07, 2022 at 12:23:03AM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 7, 2022 at 12:19 AM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Mon, Mar 7, 2022 at 12:11 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >
> > > > The ngpio fields both in struct gpio_device as well as gpio_chip are
> > > > 16-bit unsigned integers. Let's not risk an overflow and check if the
> > > > property value represented as a 32-bit unsigned integer is not greater
> > > > than U16_MAX.
> > >
> > > ...
> > >
> > > > + if (ngpios > U16_MAX) {
> > > > + ret = EINVAL;
> > > > + goto err_free_descs;
> > > > + }
> > >
> > > I don't think it's a fatal error in this case. I would perhaps print a
> > > warning and simply use a masked (which is done implicitly by an
> > > assignment of the different type) value. Note, the above is buggy on
> > > the buggy DTs, where the upper part of the value is not used. After
> > > this patch you effectively make a regression on, yes, broken DTs.
> >
> > Like
> >
> > if (ngpios > U16_MAX)
> > chip_warn(gc, "line cnt %u is greater than supported; use
> > %u\n", ngpios, (u16)ngpio);
>
> Or to be on safer side move this after == 0 check as
>
> if (gc->ngpio != ngpios)
> chip_warn(gc, "line cnt %u is greater than supported; use %u\n", ngpios, gc->ngpio);
>
ngpios is not necessarily used so this check must be in the scope of
the device property read (inside the if (gc->ngpio == 0) { block).
Bart
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gpiolib: check for overflow when reading the 'ngpios' property
2022-03-07 10:08 ` Bartosz Golaszewski
@ 2022-03-07 10:23 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2022-03-07 10:23 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List
On Mon, Mar 07, 2022 at 11:08:51AM +0100, Bartosz Golaszewski wrote:
> On Mon, Mar 7, 2022 at 10:53 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Mar 07, 2022 at 12:23:03AM +0200, Andy Shevchenko wrote:
> > > On Mon, Mar 7, 2022 at 12:19 AM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Mon, Mar 7, 2022 at 12:11 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > > > The ngpio fields both in struct gpio_device as well as gpio_chip are
> > > > > 16-bit unsigned integers. Let's not risk an overflow and check if the
> > > > > property value represented as a 32-bit unsigned integer is not greater
> > > > > than U16_MAX.
> > > >
> > > > ...
> > > >
> > > > > + if (ngpios > U16_MAX) {
> > > > > + ret = EINVAL;
> > > > > + goto err_free_descs;
> > > > > + }
> > > >
> > > > I don't think it's a fatal error in this case. I would perhaps print a
> > > > warning and simply use a masked (which is done implicitly by an
> > > > assignment of the different type) value. Note, the above is buggy on
> > > > the buggy DTs, where the upper part of the value is not used. After
> > > > this patch you effectively make a regression on, yes, broken DTs.
> > >
> > > Like
> > >
> > > if (ngpios > U16_MAX)
> > > chip_warn(gc, "line cnt %u is greater than supported; use
> > > %u\n", ngpios, (u16)ngpio);
> >
> > Or to be on safer side move this after == 0 check as
> >
> > if (gc->ngpio != ngpios)
> > chip_warn(gc, "line cnt %u is greater than supported; use %u\n", ngpios, gc->ngpio);
> >
>
> ngpios is not necessarily used so this check must be in the scope of
> the device property read (inside the if (gc->ngpio == 0) { block).
Can be done as
if (gc->ngpio == 0) {
...
} else {
ngpios = gc->ngpio;
}
if (gc->ngpio == 0) {
...
}
if (gc->ngpio != ngpios)
chip_warn(gc, "line cnt %u is greater than supported; use %u\n", ngpios, gc->ngpio);
The point of this exercise is to avoid hard coded type of the variable in a
few places, so if gc->ngpio and/or ngpios have changed type in the future,
you don't need to change this code.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-07 11:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-06 19:34 [PATCH] gpiolib: check for overflow when reading the 'ngpios' property Bartosz Golaszewski
2022-03-06 22:19 ` Andy Shevchenko
2022-03-06 22:23 ` Andy Shevchenko
2022-03-07 9:53 ` Andy Shevchenko
2022-03-07 10:08 ` Bartosz Golaszewski
2022-03-07 10:23 ` Andy Shevchenko
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).