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: <CAK8P3a1tozFc7Rmmv04dovtieHcLpg1VHaeZpW_NKFw4w-fZug@mail.gmail.com>
Date:   Fri, 14 Jul 2017 21:59:50 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Networking <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 <linux-scsi@...r.kernel.org>,
        "the arch/x86 maintainers" <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,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers

On Fri, Jul 14, 2017 at 2:52 PM, Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
> 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.

Makes sense. I didn't remember the syntax for 2) and couldn't find
it in the man page when I first looked. This seems like a good solution
here.

I'm pretty sure I tried 3) a few times when the warning first showed
up last year, but couldn't get that to work. Filing a gcc bug also seems
like a good idea, but I should first see if it's already fixed. The version
I use for testing at the moment is from late April, and others may
have complained about that already.

      Arnd