[PATCH] iio:core: Fix bug in length of event info_mask and catch unhandled bits set in masks.

[PATCH] iio:core: Fix bug in length of event info_mask and catch unhandled bits set in masks.

[Young Xiao]

The incorrect limit for the for_each_set_bit loop was noticed whilst fixing
this other case.  Note that as we only have 3 possible entries a the moment
and the value was set to 4, the bug would not have any effect currently.
It will bite fairly soon though, so best fix it now.

See commit ef4b4856593f ("iio:core: Fix bug in length of event info_mask and
catch unhandled bits set in masks.") for details.

Signed-off-by: Young Xiao <92siuyang@gmail.com>
---
 drivers/iio/industrialio-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index f5a4581..dd8873a 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1107,6 +1107,8 @@ static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
 	char *avail_postfix;
 
 	for_each_set_bit(i, infomask, sizeof(*infomask) * 8) {
+		if (i >= ARRAY_SIZE(iio_chan_info_postfix))
+			return -EINVAL;
 		avail_postfix = kasprintf(GFP_KERNEL,
 					  "%s_available",
 					  iio_chan_info_postfix[i]);
-- 
2.7.4

Re: [PATCH] iio:core: Fix bug in length of event info_mask and catch unhandled bits set in masks.

[Ardelean, Alexandru]

On Tue, 2019-06-04 at 20:40 +0800, Young Xiao wrote:
> [External]
> 
> 
> The incorrect limit for the for_each_set_bit loop was noticed whilst fixing
> this other case.  Note that as we only have 3 possible entries a the moment
> and the value was set to 4, the bug would not have any effect currently.
> It will bite fairly soon though, so best fix it now.
> 
> See commit ef4b4856593f ("iio:core: Fix bug in length of event info_mask and
> catch unhandled bits set in masks.") for details.
> 
> Signed-off-by: Young Xiao <92siuyang@gmail.com>

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Thanks for this patch.
This fix is validated also by the fact that iio_device_add_info_mask_type() has this check on the same iteration.


> ---
>  drivers/iio/industrialio-core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index f5a4581..dd8873a 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1107,6 +1107,8 @@ static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
>         char *avail_postfix;
> 
>         for_each_set_bit(i, infomask, sizeof(*infomask) * 8) {
> +               if (i >= ARRAY_SIZE(iio_chan_info_postfix))
> +                       return -EINVAL;
>                 avail_postfix = kasprintf(GFP_KERNEL,
>                                           "%s_available",
>                                           iio_chan_info_postfix[i]);
> --
> 2.7.4
> 

Re: [PATCH] iio:core: Fix bug in length of event info_mask and catch unhandled bits set in masks.

[Jonathan Cameron]

On Thu, 6 Jun 2019 08:59:10 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Tue, 2019-06-04 at 20:40 +0800, Young Xiao wrote:
> > [External]
> > 
> > 
> > The incorrect limit for the for_each_set_bit loop was noticed whilst fixing
> > this other case.  Note that as we only have 3 possible entries a the moment
> > and the value was set to 4, the bug would not have any effect currently.
> > It will bite fairly soon though, so best fix it now.
> > 
> > See commit ef4b4856593f ("iio:core: Fix bug in length of event info_mask and
> > catch unhandled bits set in masks.") for details.
> > 
> > Signed-off-by: Young Xiao <92siuyang@gmail.com>  
> 
> Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> 
> Thanks for this patch.
> This fix is validated also by the fact that iio_device_add_info_mask_type() has this check on the same iteration.

I don't think it is technically a bug, as the higher bits should never be set.
Still it is a sensible bit of hardening so applied to the togreg branch of iio.git
and pushed out as testing.

Thanks

Jonathan


> 
> 
> > ---
> >  drivers/iio/industrialio-core.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index f5a4581..dd8873a 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -1107,6 +1107,8 @@ static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
> >         char *avail_postfix;
> > 
> >         for_each_set_bit(i, infomask, sizeof(*infomask) * 8) {
> > +               if (i >= ARRAY_SIZE(iio_chan_info_postfix))
> > +                       return -EINVAL;
> >                 avail_postfix = kasprintf(GFP_KERNEL,
> >                                           "%s_available",
> >                                           iio_chan_info_postfix[i]);
> > --
> > 2.7.4
> >