From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnd Bergmann Subject: Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers Date: Fri, 14 Jul 2017 21:59:50 +0200 Message-ID: References: <20170714120720.906842-1-arnd@arndb.de> <20170714120720.906842-19-arnd@arndb.de> <1500036740.29303.7.camel@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: <1500036740.29303.7.camel@linux.intel.com> Sender: linux-gpio-owner@vger.kernel.org To: Andy Shevchenko Cc: Linux Kernel Mailing List , Mika Westerberg , Linus Walleij , Greg Kroah-Hartman , Linus Torvalds , Guenter Roeck , Andrew Morton , Networking , "David S . Miller" , "James E . J . Bottomley" , "Martin K . Petersen" , linux-scsi , the arch/x86 maintainers , Hans de Goede , "Rafael J. Wysocki" , Wei Yongjun , linux-gpio@vger.kernel.org, ACPI List-Id: linux-acpi@vger.kernel.org On Fri, Jul 14, 2017 at 2:52 PM, Andy Shevchenko 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751215AbdGNT7z (ORCPT ); Fri, 14 Jul 2017 15:59:55 -0400 Received: from mail-oi0-f65.google.com ([209.85.218.65]:33732 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751019AbdGNT7w (ORCPT ); Fri, 14 Jul 2017 15:59:52 -0400 MIME-Version: 1.0 In-Reply-To: <1500036740.29303.7.camel@linux.intel.com> References: <20170714120720.906842-1-arnd@arndb.de> <20170714120720.906842-19-arnd@arndb.de> <1500036740.29303.7.camel@linux.intel.com> From: Arnd Bergmann Date: Fri, 14 Jul 2017 21:59:50 +0200 X-Google-Sender-Auth: KshI4G013ygz62fEki43Ec73oFs Message-ID: Subject: Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers To: Andy Shevchenko Cc: Linux Kernel Mailing List , Mika Westerberg , Linus Walleij , Greg Kroah-Hartman , Linus Torvalds , Guenter Roeck , Andrew Morton , Networking , "David S . Miller" , "James E . J . Bottomley" , "Martin K . Petersen" , linux-scsi , "the arch/x86 maintainers" , Hans de Goede , "Rafael J. Wysocki" , Wei Yongjun , linux-gpio@vger.kernel.org, ACPI Devel Maling List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jul 14, 2017 at 2:52 PM, Andy Shevchenko 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnd Bergmann Subject: Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers Date: Fri, 14 Jul 2017 21:59:50 +0200 Message-ID: References: <20170714120720.906842-1-arnd@arndb.de> <20170714120720.906842-19-arnd@arndb.de> <1500036740.29303.7.camel@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Linux Kernel Mailing List , Mika Westerberg , Linus Walleij , Greg Kroah-Hartman , Linus Torvalds , Guenter Roeck , Andrew Morton , Networking , "David S . Miller" , "James E . J . Bottomley" , "Martin K . Petersen" , linux-scsi , "the arch/x86 maintainers" , Hans de Goede , "Rafael J. Wysocki" , Wei Yongjun , linux-gpio@vger.kernel.org, ACPI Devel Maling L To: Andy Shevchenko Return-path: In-Reply-To: <1500036740.29303.7.camel@linux.intel.com> Sender: linux-gpio-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Fri, Jul 14, 2017 at 2:52 PM, Andy Shevchenko 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