[PATCH v2] iio: irsd200: fix -Warray-bounds bug in irsd200_trigger_handler

[PATCH v2] iio: irsd200: fix -Warray-bounds bug in irsd200_trigger_handler

[GONG, Ruiqi]

From: "GONG, Ruiqi" <gongruiqi1@huawei.com>

When compiling with gcc 13 with -Warray-bounds enabled:

In file included from drivers/iio/proximity/irsd200.c:15:
In function ‘iio_push_to_buffers_with_timestamp’,
    inlined from ‘irsd200_trigger_handler’ at drivers/iio/proximity/irsd200.c:770:2:
./include/linux/iio/buffer.h:42:46: error: array subscript ‘int64_t {aka long long int}[0]’
is partly outside array bounds of ‘s16[1]’ {aka ‘short int[1]’} [-Werror=array-bounds=]
   42 |                 ((int64_t *)data)[ts_offset] = timestamp;
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
drivers/iio/proximity/irsd200.c: In function ‘irsd200_trigger_handler’:
drivers/iio/proximity/irsd200.c:763:13: note: object ‘buf’ of size 2
  763 |         s16 buf = 0;
      |             ^~~

The problem seems to be that irsd200_trigger_handler() is taking a s16
variable as an int64_t buffer. As Jonathan suggested [1], fix it by
extending the buffer to a two-element array of s64.

Link: https://github.com/KSPP/linux/issues/331
Link: https://lore.kernel.org/lkml/20230809181329.46c00a5d@jic23-huawei/ [1]
Fixes: 3db3562bc66e ("iio: Add driver for Murata IRS-D200")
Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com>
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---

v2: change buf to `s64 buf[2]`

 drivers/iio/proximity/irsd200.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/proximity/irsd200.c b/drivers/iio/proximity/irsd200.c
index 5bd791b46d98..bdff91f6b1a3 100644
--- a/drivers/iio/proximity/irsd200.c
+++ b/drivers/iio/proximity/irsd200.c
@@ -759,14 +759,14 @@ static irqreturn_t irsd200_trigger_handler(int irq, void *pollf)
 {
 	struct iio_dev *indio_dev = ((struct iio_poll_func *)pollf)->indio_dev;
 	struct irsd200_data *data = iio_priv(indio_dev);
-	s16 buf = 0;
+	s64 buf[2] = {};
 	int ret;
 
-	ret = irsd200_read_data(data, &buf);
+	ret = irsd200_read_data(data, (s16 *)buf);
 	if (ret)
 		goto end;
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &buf,
+	iio_push_to_buffers_with_timestamp(indio_dev, buf,
 					   iio_get_time_ns(indio_dev));
 
 end:
-- 
2.41.0

Re: [PATCH v2] iio: irsd200: fix -Warray-bounds bug in irsd200_trigger_handler

[Kees Cook]

On Thu, Aug 10, 2023 at 11:59:10AM +0800, GONG, Ruiqi wrote:
> From: "GONG, Ruiqi" <gongruiqi1@huawei.com>
> 
> When compiling with gcc 13 with -Warray-bounds enabled:
> 
> In file included from drivers/iio/proximity/irsd200.c:15:
> In function ‘iio_push_to_buffers_with_timestamp’,
>     inlined from ‘irsd200_trigger_handler’ at drivers/iio/proximity/irsd200.c:770:2:
> ./include/linux/iio/buffer.h:42:46: error: array subscript ‘int64_t {aka long long int}[0]’
> is partly outside array bounds of ‘s16[1]’ {aka ‘short int[1]’} [-Werror=array-bounds=]
>    42 |                 ((int64_t *)data)[ts_offset] = timestamp;
>       |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> drivers/iio/proximity/irsd200.c: In function ‘irsd200_trigger_handler’:
> drivers/iio/proximity/irsd200.c:763:13: note: object ‘buf’ of size 2
>   763 |         s16 buf = 0;
>       |             ^~~
> 
> The problem seems to be that irsd200_trigger_handler() is taking a s16
> variable as an int64_t buffer. As Jonathan suggested [1], fix it by
> extending the buffer to a two-element array of s64.
> 
> Link: https://github.com/KSPP/linux/issues/331
> Link: https://lore.kernel.org/lkml/20230809181329.46c00a5d@jic23-huawei/ [1]
> Fixes: 3db3562bc66e ("iio: Add driver for Murata IRS-D200")
> Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH v2] iio: irsd200: fix -Warray-bounds bug in irsd200_trigger_handler

[Waqar Hameed]

On Thu, Aug 10, 2023 at 11:59 +0800 "GONG, Ruiqi" <gongruiqi@huaweicloud.com> wrote:

> From: "GONG, Ruiqi" <gongruiqi1@huawei.com>
>
> When compiling with gcc 13 with -Warray-bounds enabled:
>
> In file included from drivers/iio/proximity/irsd200.c:15:
> In function ‘iio_push_to_buffers_with_timestamp’,
>     inlined from ‘irsd200_trigger_handler’ at drivers/iio/proximity/irsd200.c:770:2:
> ./include/linux/iio/buffer.h:42:46: error: array subscript ‘int64_t {aka long long int}[0]’
> is partly outside array bounds of ‘s16[1]’ {aka ‘short int[1]’} [-Werror=array-bounds=]
>    42 |                 ((int64_t *)data)[ts_offset] = timestamp;
>       |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> drivers/iio/proximity/irsd200.c: In function ‘irsd200_trigger_handler’:
> drivers/iio/proximity/irsd200.c:763:13: note: object ‘buf’ of size 2
>   763 |         s16 buf = 0;
>       |             ^~~
>
> The problem seems to be that irsd200_trigger_handler() is taking a s16
> variable as an int64_t buffer. As Jonathan suggested [1], fix it by
> extending the buffer to a two-element array of s64.
>
> Link: https://github.com/KSPP/linux/issues/331
> Link: https://lore.kernel.org/lkml/20230809181329.46c00a5d@jic23-huawei/ [1]
> Fixes: 3db3562bc66e ("iio: Add driver for Murata IRS-D200")
> Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com>
> Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Reviewed-by: Waqar Hameed <waqar.hameed@axis.com>
Tested-by: Waqar Hameed <waqar.hameed@axis.com>

Re: [PATCH v2] iio: irsd200: fix -Warray-bounds bug in irsd200_trigger_handler

[Jonathan Cameron]

On Mon, 21 Aug 2023 15:41:18 +0200
Waqar Hameed <waqar.hameed@axis.com> wrote:

> On Thu, Aug 10, 2023 at 11:59 +0800 "GONG, Ruiqi" <gongruiqi@huaweicloud.com> wrote:
> 
> > From: "GONG, Ruiqi" <gongruiqi1@huawei.com>
> >
> > When compiling with gcc 13 with -Warray-bounds enabled:
> >
> > In file included from drivers/iio/proximity/irsd200.c:15:
> > In function ‘iio_push_to_buffers_with_timestamp’,
> >     inlined from ‘irsd200_trigger_handler’ at drivers/iio/proximity/irsd200.c:770:2:
> > ./include/linux/iio/buffer.h:42:46: error: array subscript ‘int64_t {aka long long int}[0]’
> > is partly outside array bounds of ‘s16[1]’ {aka ‘short int[1]’} [-Werror=array-bounds=]
> >    42 |                 ((int64_t *)data)[ts_offset] = timestamp;
> >       |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> > drivers/iio/proximity/irsd200.c: In function ‘irsd200_trigger_handler’:
> > drivers/iio/proximity/irsd200.c:763:13: note: object ‘buf’ of size 2
> >   763 |         s16 buf = 0;
> >       |             ^~~
> >
> > The problem seems to be that irsd200_trigger_handler() is taking a s16
> > variable as an int64_t buffer. As Jonathan suggested [1], fix it by
> > extending the buffer to a two-element array of s64.
> >
> > Link: https://github.com/KSPP/linux/issues/331
> > Link: https://lore.kernel.org/lkml/20230809181329.46c00a5d@jic23-huawei/ [1]
> > Fixes: 3db3562bc66e ("iio: Add driver for Murata IRS-D200")
> > Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com>
> > Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>  
> 
> Reviewed-by: Waqar Hameed <waqar.hameed@axis.com>
> Tested-by: Waqar Hameed <waqar.hameed@axis.com>

Applied to the fixes-togreg branch of iio.git.  However as we are mid
merge window that tree has an odd base and I'll wait to rebase it on rc1 before
sending a pull request.

Thanks,

Jonathan