[PATCH 0/2] Input: usbtouchscreen - suppress empty array warnings

[PATCH 0/2] Input: usbtouchscreen - suppress empty array warnings

[Johan Hovold]

I ran into some new (?) compiler warnings with -rc3 this morning. Not
sure if it's after updating to GCC 11.3.0 a few weeks ago, or due to
some other change.

Either way, making sure that the device info table is non empty is
straight forward and the sentinel value can also be used to protect
against potential future bugs due to the somewhat fragile driver
ifdeffery.

Johan


Johan Hovold (2):
  Input: usbtouchscreen - suppress empty array warnings
  Input: usbtouchscreen - add driver_info sanity check

 drivers/input/touchscreen/usbtouchscreen.c | 5 +++++
 1 file changed, 5 insertions(+)

-- 
2.35.1

[PATCH 1/2] Input: usbtouchscreen - suppress empty array warnings

[Johan Hovold]

When compile-testing the USB touchscreen driver without enabling any of
the device type options the usbtouch_dev_info array ends up being empty,
something which triggers compiler warning with -Warray-bounds
(gcc-11.3.0).

drivers/input/touchscreen/usbtouchscreen.c: In function 'usbtouch_probe':
drivers/input/touchscreen/usbtouchscreen.c:1668:16:warning: array subscript <unknown> is outside array bounds of 'struct usbtouch_device_info[0]' [-Warray-bounds]
 1668 |         type = &usbtouch_dev_info[id->driver_info];

Suppress the warnings by making sure that the array is always non-empty.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/input/touchscreen/usbtouchscreen.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index 43c521f50c85..6683554f0e92 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -128,6 +128,7 @@ enum {
 	DEVTYPE_NEXIO,
 	DEVTYPE_ELO,
 	DEVTYPE_ETOUCH,
+	DEVTYPE_COUNT
 };
 
 #define USB_DEVICE_HID_CLASS(vend, prod) \
@@ -1379,6 +1380,7 @@ static struct usbtouch_device_info usbtouch_dev_info[] = {
 		.read_data	= etouch_read_data,
 	},
 #endif
+	[DEVTYPE_COUNT] = { }	/* Make sure array is non-empty */
 };
 
 
-- 
2.35.1

Re: [PATCH 1/2] Input: usbtouchscreen - suppress empty array warnings

[Dmitry Torokhov]

Hi Johan,

On Mon, Jun 20, 2022 at 10:46:27AM +0200, Johan Hovold wrote:
> When compile-testing the USB touchscreen driver without enabling any of
> the device type options the usbtouch_dev_info array ends up being empty,
> something which triggers compiler warning with -Warray-bounds
> (gcc-11.3.0).
> 
> drivers/input/touchscreen/usbtouchscreen.c: In function 'usbtouch_probe':
> drivers/input/touchscreen/usbtouchscreen.c:1668:16:warning: array subscript <unknown> is outside array bounds of 'struct usbtouch_device_info[0]' [-Warray-bounds]
>  1668 |         type = &usbtouch_dev_info[id->driver_info];
> 
> Suppress the warnings by making sure that the array is always non-empty.

Does it still warn if you add a check for type, something like

	if (type >= ARRAY_SIZE(usbtouch_device_info))
		return -ENODEV;

?

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] Input: usbtouchscreen - suppress empty array warnings

[Johan Hovold]

On Wed, Jun 22, 2022 at 04:22:08PM -0700, Dmitry Torokhov wrote:
> Hi Johan,
> 
> On Mon, Jun 20, 2022 at 10:46:27AM +0200, Johan Hovold wrote:
> > When compile-testing the USB touchscreen driver without enabling any of
> > the device type options the usbtouch_dev_info array ends up being empty,
> > something which triggers compiler warning with -Warray-bounds
> > (gcc-11.3.0).
> > 
> > drivers/input/touchscreen/usbtouchscreen.c: In function 'usbtouch_probe':
> > drivers/input/touchscreen/usbtouchscreen.c:1668:16:warning: array subscript <unknown> is outside array bounds of 'struct usbtouch_device_info[0]' [-Warray-bounds]
> >  1668 |         type = &usbtouch_dev_info[id->driver_info];
> > 
> > Suppress the warnings by making sure that the array is always non-empty.
> 
> Does it still warn if you add a check for type, something like
> 
> 	if (type >= ARRAY_SIZE(usbtouch_device_info))
> 		return -ENODEV;
> 
> ?

It seems 

	if (id->driver_info >= ARRAY_SIZE(usbtouch_dev_info))
                return -ENODEV;

indeed does the trick. I'll send a v2.

Johan

[PATCH 2/2] Input: usbtouchscreen - add driver_info sanity check

[Johan Hovold]

Add a sanity check on the device id-table driver_info field to make sure
we never access a type structure (and function pointers) outside of the
device info array (e.g. if someone fails to ifdef a device-id entry).

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/input/touchscreen/usbtouchscreen.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index 6683554f0e92..f90acbeea74f 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -1656,6 +1656,9 @@ static int usbtouch_probe(struct usb_interface *intf,
 	if (id->driver_info == DEVTYPE_IGNORE)
 		return -ENODEV;
 
+	if (id->driver_info >= DEVTYPE_COUNT)
+		return -EINVAL;
+
 	endpoint = usbtouch_get_input_endpoint(intf->cur_altsetting);
 	if (!endpoint)
 		return -ENXIO;
-- 
2.35.1