linux-kernel - Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1500036740.29303.7.camel@linux.intel.com>
Date:   Fri, 14 Jul 2017 15:52:20 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Linus Walleij <linus.walleij@...aro.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Guenter Roeck <linux@...ck-us.net>, akpm@...ux-foundation.org,
        netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
        "James E . J . Bottomley" <jejb@...ux.vnet.ibm.com>,
        "Martin K . Petersen" <martin.petersen@...cle.com>,
        linux-scsi@...r.kernel.org, x86@...nel.org,
        Hans de Goede <hdegoede@...hat.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Wei Yongjun <weiyongjun1@...wei.com>,
        linux-gpio@...r.kernel.org, linux-acpi@...r.kernel.org
Subject: Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin
 numbers

On Fri, 2017-07-14 at 14:07 +0200, Arnd Bergmann wrote:
> gcc-7 notices that the pin_table is an array of 16-bit numbers,
> but we assume it can be printed as a two-character hexadecimal
> string:
> 
> 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);
>     ~~~~


This is obviously a false positive warning.

Here we have
int pin = u16 pin_table[0] <= 255 (implying >= 0).

I see few options how to make it more clear
1) your proposal;
2) use "%02hhX" instead;
3) use if (ret >= 0 && ret <= 255) condition.

I would choose one of the 2-3.

In case gcc will complain about 3), file a bug to gcc crazy warning.

> 
> This can't be right, so this changes it to truncate the number to
> an 8-bit pin number.
> 
> Fixes: 0d1c28a449c6 ("gpiolib-acpi: Add ACPI5 event model support to
> gpio.")
> Signed-off-by: Arnd Bergmann <arnd@...db.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 c9b42dd12dfa..c3faea724af8 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -205,7 +205,7 @@ static acpi_status
> acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
>  		char ev_name[5];
>  		sprintf(ev_name, "_%c%02X",
>  			agpio->triggering == ACPI_EDGE_SENSITIVE ?
> 'E' : 'L',
> -			pin);
> +			(u8)pin);
>  		if (ACPI_SUCCESS(acpi_get_handle(handle, ev_name,
> &evt_handle)))
>  			handler = acpi_gpio_irq_handler;
>  	}

-- 
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Intel Finland Oy