[PATCH 1/2] iio: fix possible buffer overflow

[PATCH 1/2] iio: fix possible buffer overflow

[Alexandre Belloni]

Found using smatch:
drivers/iio/industrialio-core.c:719 iio_device_add_info_mask_type() error:
buffer overflow 'iio_chan_info_postfix' 17 <= 63

It was probably never hit because the info_mask_* members are filled by using
the BIT() macro with values from the iio_chan_info_enum enum that also serve as
the index of the iio_chan_info_postfix array.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/iio/industrialio-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index ede16aec20fb..5e7a67e53879 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -715,7 +715,8 @@ static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
 {
 	int i, ret, attrcount = 0;
 
-	for_each_set_bit(i, infomask, sizeof(infomask)*8) {
+	for_each_set_bit(i, infomask, min(sizeof(infomask)*8,
+					  ARRAY_SIZE(iio_chan_info_postfix))) {
 		ret = __iio_add_chan_devattr(iio_chan_info_postfix[i],
 					     chan,
 					     &iio_read_channel_info,
-- 
1.9.1

[PATCH 2/2] iio: fix possible buffer overflow

[Alexandre Belloni]

Found using smatch:
drivers/iio/industrialio-event.c:327 iio_device_add_event() error: buffer
overflow 'iio_ev_info_text' 3 <= 7

It was probably never hit because the mask_* members of the event_spec struct
are filled by using the BIT() macro with values from the iio_event_info enum
that also serve as the index of the iio_ev_info_text array.

Also, for_each_set_bit takes a number of bits as the size, not a number of
bytes.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/iio/industrialio-event.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index ea6e06b9c7d4..804e90676159 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -321,7 +321,8 @@ static int iio_device_add_event(struct iio_dev *indio_dev,
 	char *postfix;
 	int ret;
 
-	for_each_set_bit(i, mask, sizeof(*mask)) {
+	for_each_set_bit(i, mask,
+			 min(sizeof(*mask)*8, ARRAY_SIZE(iio_ev_info_text))) {
 		postfix = kasprintf(GFP_KERNEL, "%s_%s_%s",
 				iio_ev_type_text[type], iio_ev_dir_text[dir],
 				iio_ev_info_text[i]);
-- 
1.9.1

Re: [PATCH 1/2] iio: fix possible buffer overflow

[Jonathan Cameron]

On 02/05/14 23:40, Alexandre Belloni wrote:
> Found using smatch:
> drivers/iio/industrialio-core.c:719 iio_device_add_info_mask_type() error:
> buffer overflow 'iio_chan_info_postfix' 17 <= 63
>
> It was probably never hit because the info_mask_* members are filled by using
> the BIT() macro with values from the iio_chan_info_enum enum that also serve as
> the index of the iio_chan_info_postfix array.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
See
ef4b4856593fc3d9d169bededdaf7acf62f83a52
iio:core: Fix bug in length of event info_mask and catch unhandled bits set in masks.

Which fixes the same issue in a slightly different way.

Pretty recent patch though and this was there for ages before that.
Better to have two fixes than none.

Thanks,

J
> ---
>   drivers/iio/industrialio-core.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index ede16aec20fb..5e7a67e53879 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -715,7 +715,8 @@ static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
>   {
>   	int i, ret, attrcount = 0;
>
> -	for_each_set_bit(i, infomask, sizeof(infomask)*8) {
> +	for_each_set_bit(i, infomask, min(sizeof(infomask)*8,
> +					  ARRAY_SIZE(iio_chan_info_postfix))) {
>   		ret = __iio_add_chan_devattr(iio_chan_info_postfix[i],
>   					     chan,
>   					     &iio_read_channel_info,
>

Re: [PATCH 1/2] iio: fix possible buffer overflow

[Alexandre Belloni]

On 03/05/2014 at 11:11:50 +0100, Jonathan Cameron wrote :
> On 02/05/14 23:40, Alexandre Belloni wrote:
> >Found using smatch:
> >drivers/iio/industrialio-core.c:719 iio_device_add_info_mask_type() error:
> >buffer overflow 'iio_chan_info_postfix' 17 <= 63
> >
> >It was probably never hit because the info_mask_* members are filled by using
> >the BIT() macro with values from the iio_chan_info_enum enum that also serve as
> >the index of the iio_chan_info_postfix array.
> >
> >Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> See
> ef4b4856593fc3d9d169bededdaf7acf62f83a52
> iio:core: Fix bug in length of event info_mask and catch unhandled bits set in masks.
> 
> Which fixes the same issue in a slightly different way.
> 
> Pretty recent patch though and this was there for ages before that.
> Better to have two fixes than none.
> 

Yeah, I missed your patch and it didn't hit Linus' tree yet. Sorry about
the noise, next time I'll try to remember to check your tree.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com