All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gpio: acpi: work around false-positive -Wstring-overflow warning
@ 2017-09-06 15:47 Arnd Bergmann
  2017-09-06 15:52 ` Joe Perches
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Arnd Bergmann @ 2017-09-06 15:47 UTC (permalink / raw)
  To: Mika Westerberg, Andy Shevchenko, Linus Walleij
  Cc: Arnd Bergmann, Dmitry Torokhov, Hans de Goede, Wei Yongjun,
	linux-gpio, linux-acpi, linux-kernel

gcc-7 notices that the pin_table is an array of 16-bit numbers,
but fails to take the following range check into account:

drivers/gpio/gpiolib-acpi.c: In function 'acpi_gpiochip_request_interrupt':
drivers/gpio/gpiolib-acpi.c:206:24: warning: '%02X' directive writing between 2 and 4 bytes into a region of size 3 [-Wformat-overflow=]
   sprintf(ev_name, "_%c%02X",
                        ^~~~
drivers/gpio/gpiolib-acpi.c:206:20: note: directive argument in the range [0, 65535]
   sprintf(ev_name, "_%c%02X",
                    ^~~~~~~~~
drivers/gpio/gpiolib-acpi.c:206:3: note: 'sprintf' output between 5 and 7 bytes into a destination of size 5
   sprintf(ev_name, "_%c%02X",
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
    agpio->triggering == ACPI_EDGE_SENSITIVE ? 'E' : 'L',
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    pin);
    ~~~~

As suggested by Andy, this changes the format string to have a fixed length.
Since modifying the range check did not help, I also opened a bug against
gcc, see link below.

Fixes: 0d1c28a449c6 ("gpiolib-acpi: Add ACPI5 event model support to gpio.")
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://patchwork.kernel.org/patch/9840801/
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82123
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpio/gpiolib-acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 4d2113530735..eb4528c87c0b 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -203,7 +203,7 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
 
 	if (pin <= 255) {
 		char ev_name[5];
-		sprintf(ev_name, "_%c%02X",
+		sprintf(ev_name, "_%c%02hhX",
 			agpio->triggering == ACPI_EDGE_SENSITIVE ? 'E' : 'L',
 			pin);
 		if (ACPI_SUCCESS(acpi_get_handle(handle, ev_name, &evt_handle)))
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] gpio: acpi: work around false-positive -Wstring-overflow warning
  2017-09-06 15:47 [PATCH] gpio: acpi: work around false-positive -Wstring-overflow warning Arnd Bergmann
@ 2017-09-06 15:52 ` Joe Perches
  2017-09-06 19:53   ` Arnd Bergmann
  2017-09-07  6:50 ` Mika Westerberg
  2017-09-12  9:26 ` Linus Walleij
  2 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2017-09-06 15:52 UTC (permalink / raw)
  To: Arnd Bergmann, Mika Westerberg, Andy Shevchenko, Linus Walleij
  Cc: Dmitry Torokhov, Hans de Goede, Wei Yongjun, linux-gpio,
	linux-acpi, linux-kernel

On Wed, 2017-09-06 at 17:47 +0200, Arnd Bergmann wrote:
> gcc-7 notices that the pin_table is an array of 16-bit numbers,
> but fails to take the following range check into account:
> 
> drivers/gpio/gpiolib-acpi.c: In function 'acpi_gpiochip_request_interrupt':
> drivers/gpio/gpiolib-acpi.c:206:24: warning: '%02X' directive writing between 2 and 4 bytes into a region of size 3 [-Wformat-overflow=]
>    sprintf(ev_name, "_%c%02X",
>                         ^~~~
> drivers/gpio/gpiolib-acpi.c:206:20: note: directive argument in the range [0, 65535]
>    sprintf(ev_name, "_%c%02X",
>                     ^~~~~~~~~
> drivers/gpio/gpiolib-acpi.c:206:3: note: 'sprintf' output between 5 and 7 bytes into a destination of size 5
>    sprintf(ev_name, "_%c%02X",
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>     agpio->triggering == ACPI_EDGE_SENSITIVE ? 'E' : 'L',
>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     pin);
>     ~~~~
> 
> As suggested by Andy, this changes the format string to have a fixed length.
> Since modifying the range check did not help, I also opened a bug against
> gcc, see link below.

The code has

	int pin

does changing pin to unsigned int matter?

> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
[]
> @@ -203,7 +203,7 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
>  
>  	if (pin <= 255) {
>  		char ev_name[5];
> -		sprintf(ev_name, "_%c%02X",
> +		sprintf(ev_name, "_%c%02hhX",
>  			agpio->triggering == ACPI_EDGE_SENSITIVE ? 'E' : 'L',
>  			pin);
>  		if (ACPI_SUCCESS(acpi_get_handle(handle, ev_name, &evt_handle)))

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gpio: acpi: work around false-positive -Wstring-overflow warning
  2017-09-06 15:52 ` Joe Perches
@ 2017-09-06 19:53   ` Arnd Bergmann
  2017-09-06 23:42     ` Joe Perches
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2017-09-06 19:53 UTC (permalink / raw)
  To: Joe Perches
  Cc: Mika Westerberg, Andy Shevchenko, Linus Walleij, Dmitry Torokhov,
	Hans de Goede, Wei Yongjun, linux-gpio, ACPI Devel Maling List,
	Linux Kernel Mailing List

On Wed, Sep 6, 2017 at 5:52 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2017-09-06 at 17:47 +0200, Arnd Bergmann wrote:

>> As suggested by Andy, this changes the format string to have a fixed length.
>> Since modifying the range check did not help, I also opened a bug against
>> gcc, see link below.
>
> The code has
>
>         int pin
>
> does changing pin to unsigned int matter?

No difference, see also the gcc bugzilla report.

        Arnd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gpio: acpi: work around false-positive -Wstring-overflow warning
  2017-09-06 19:53   ` Arnd Bergmann
@ 2017-09-06 23:42     ` Joe Perches
  0 siblings, 0 replies; 6+ messages in thread
From: Joe Perches @ 2017-09-06 23:42 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mika Westerberg, Andy Shevchenko, Linus Walleij, Dmitry Torokhov,
	Hans de Goede, Wei Yongjun, linux-gpio, ACPI Devel Maling List,
	Linux Kernel Mailing List

On Wed, 2017-09-06 at 21:53 +0200, Arnd Bergmann wrote:
> On Wed, Sep 6, 2017 at 5:52 PM, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2017-09-06 at 17:47 +0200, Arnd Bergmann wrote:
> > > As suggested by Andy, this changes the format string to have a fixed length.
> > > Since modifying the range check did not help, I also opened a bug against
> > > gcc, see link below.
> > 
> > The code has
> > 
> >         int pin
> > 
> > does changing pin to unsigned int matter?
> 
> No difference, see also the gcc bugzilla report.

Perhaps the bug report does not apply here as the
function is casting a unsigned int to an int through
a pointer when the int could now be negative, the int
is tested for < 256 and is then printed.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gpio: acpi: work around false-positive -Wstring-overflow warning
  2017-09-06 15:47 [PATCH] gpio: acpi: work around false-positive -Wstring-overflow warning Arnd Bergmann
  2017-09-06 15:52 ` Joe Perches
@ 2017-09-07  6:50 ` Mika Westerberg
  2017-09-12  9:26 ` Linus Walleij
  2 siblings, 0 replies; 6+ messages in thread
From: Mika Westerberg @ 2017-09-07  6:50 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andy Shevchenko, Linus Walleij, Dmitry Torokhov, Hans de Goede,
	Wei Yongjun, linux-gpio, linux-acpi, linux-kernel

On Wed, Sep 06, 2017 at 05:47:45PM +0200, Arnd Bergmann wrote:
> gcc-7 notices that the pin_table is an array of 16-bit numbers,
> but fails to take the following range check into account:
> 
> drivers/gpio/gpiolib-acpi.c: In function 'acpi_gpiochip_request_interrupt':
> drivers/gpio/gpiolib-acpi.c:206:24: warning: '%02X' directive writing between 2 and 4 bytes into a region of size 3 [-Wformat-overflow=]
>    sprintf(ev_name, "_%c%02X",
>                         ^~~~
> drivers/gpio/gpiolib-acpi.c:206:20: note: directive argument in the range [0, 65535]
>    sprintf(ev_name, "_%c%02X",
>                     ^~~~~~~~~
> drivers/gpio/gpiolib-acpi.c:206:3: note: 'sprintf' output between 5 and 7 bytes into a destination of size 5
>    sprintf(ev_name, "_%c%02X",
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>     agpio->triggering == ACPI_EDGE_SENSITIVE ? 'E' : 'L',
>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     pin);
>     ~~~~
> 
> As suggested by Andy, this changes the format string to have a fixed length.
> Since modifying the range check did not help, I also opened a bug against
> gcc, see link below.
> 
> Fixes: 0d1c28a449c6 ("gpiolib-acpi: Add ACPI5 event model support to gpio.")
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Link: https://patchwork.kernel.org/patch/9840801/
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82123
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gpio: acpi: work around false-positive -Wstring-overflow warning
  2017-09-06 15:47 [PATCH] gpio: acpi: work around false-positive -Wstring-overflow warning Arnd Bergmann
  2017-09-06 15:52 ` Joe Perches
  2017-09-07  6:50 ` Mika Westerberg
@ 2017-09-12  9:26 ` Linus Walleij
  2 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2017-09-12  9:26 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mika Westerberg, Andy Shevchenko, Dmitry Torokhov, Hans de Goede,
	Wei Yongjun, linux-gpio, ACPI Devel Maling List, linux-kernel

On Wed, Sep 6, 2017 at 5:47 PM, Arnd Bergmann <arnd@arndb.de> wrote:

> gcc-7 notices that the pin_table is an array of 16-bit numbers,
> but fails to take the following range check into account:
>
> drivers/gpio/gpiolib-acpi.c: In function 'acpi_gpiochip_request_interrupt':
> drivers/gpio/gpiolib-acpi.c:206:24: warning: '%02X' directive writing between 2 and 4 bytes into a region of size 3 [-Wformat-overflow=]
>    sprintf(ev_name, "_%c%02X",
>                         ^~~~
> drivers/gpio/gpiolib-acpi.c:206:20: note: directive argument in the range [0, 65535]
>    sprintf(ev_name, "_%c%02X",
>                     ^~~~~~~~~
> drivers/gpio/gpiolib-acpi.c:206:3: note: 'sprintf' output between 5 and 7 bytes into a destination of size 5
>    sprintf(ev_name, "_%c%02X",
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>     agpio->triggering == ACPI_EDGE_SENSITIVE ? 'E' : 'L',
>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     pin);
>     ~~~~
>
> As suggested by Andy, this changes the format string to have a fixed length.
> Since modifying the range check did not help, I also opened a bug against
> gcc, see link below.
>
> Fixes: 0d1c28a449c6 ("gpiolib-acpi: Add ACPI5 event model support to gpio.")
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Link: https://patchwork.kernel.org/patch/9840801/
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82123
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied with Mika's ACK.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-09-12  9:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-06 15:47 [PATCH] gpio: acpi: work around false-positive -Wstring-overflow warning Arnd Bergmann
2017-09-06 15:52 ` Joe Perches
2017-09-06 19:53   ` Arnd Bergmann
2017-09-06 23:42     ` Joe Perches
2017-09-07  6:50 ` Mika Westerberg
2017-09-12  9:26 ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.