[Qemu-devel] [PULL 7/7] hw/usb: avoid format truncation warning when formatting port name

Gerd Hoffmann posted 7 patches 6 years, 9 months ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>
[Qemu-devel] [PULL 7/7] hw/usb: avoid format truncation warning when formatting port name
Posted by Gerd Hoffmann 6 years, 9 months ago
From: Daniel P. Berrangé <berrange@redhat.com>

hw/usb/hcd-xhci.c: In function ‘usb_xhci_realize’:
hw/usb/hcd-xhci.c:3339:66: warning: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Wformat-trunca\
tion=]
 3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
      |                                                                  ^~
hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
 3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
      |                                                      ^~~~~~~~~~~~~~~

The xhci code formats the port name into a fixed length
buffer which is only large enough to hold port numbers
upto 5 digits in decimal representation. We're never
going to have a port number that large, so aserting the
port number is sensible is sufficient to tell GCC the
formatted string won't be truncated.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190412121626.19829-5-berrange@redhat.com>

[ kraxel: also s/int/unsigned int/ to tell gcc they can't
          go negative. ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index d8472b4fea7f..2e9a839f2bf9 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3306,7 +3306,7 @@ static void usb_xhci_init(XHCIState *xhci)
 {
     DeviceState *dev = DEVICE(xhci);
     XHCIPort *port;
-    int i, usbports, speedmask;
+    unsigned int i, usbports, speedmask;
 
     xhci->usbsts = USBSTS_HCH;
 
@@ -3336,6 +3336,7 @@ static void usb_xhci_init(XHCIState *xhci)
                 USB_SPEED_MASK_LOW  |
                 USB_SPEED_MASK_FULL |
                 USB_SPEED_MASK_HIGH;
+            assert(i < MAXPORTS);
             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
             speedmask |= port->speedmask;
         }
@@ -3349,6 +3350,7 @@ static void usb_xhci_init(XHCIState *xhci)
             }
             port->uport = &xhci->uports[i];
             port->speedmask = USB_SPEED_MASK_SUPER;
+            assert(i < MAXPORTS);
             snprintf(port->name, sizeof(port->name), "usb3 port #%d", i+1);
             speedmask |= port->speedmask;
         }
-- 
2.18.1


Re: [Qemu-devel] [PULL 7/7] hw/usb: avoid format truncation warning when formatting port name
Posted by Philippe Mathieu-Daudé 5 years ago
Hello,

On 5/3/19 8:59 AM, Gerd Hoffmann wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> hw/usb/hcd-xhci.c: In function ‘usb_xhci_realize’:
> hw/usb/hcd-xhci.c:3339:66: warning: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Wformat-trunca\
> tion=]
>  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>       |                                                                  ^~
> hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
>  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>       |                                                      ^~~~~~~~~~~~~~~
> 
> The xhci code formats the port name into a fixed length
> buffer which is only large enough to hold port numbers
> upto 5 digits in decimal representation. We're never
> going to have a port number that large, so aserting the
> port number is sensible is sufficient to tell GCC the
> formatted string won't be truncated.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Message-Id: <20190412121626.19829-5-berrange@redhat.com>
> 
> [ kraxel: also s/int/unsigned int/ to tell gcc they can't
>           go negative. ]
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/usb/hcd-xhci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index d8472b4fea7f..2e9a839f2bf9 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -3306,7 +3306,7 @@ static void usb_xhci_init(XHCIState *xhci)
>  {
>      DeviceState *dev = DEVICE(xhci);
>      XHCIPort *port;
> -    int i, usbports, speedmask;
> +    unsigned int i, usbports, speedmask;
>  
>      xhci->usbsts = USBSTS_HCH;
>  
> @@ -3336,6 +3336,7 @@ static void usb_xhci_init(XHCIState *xhci)
>                  USB_SPEED_MASK_LOW  |
>                  USB_SPEED_MASK_FULL |
>                  USB_SPEED_MASK_HIGH;
> +            assert(i < MAXPORTS);
>              snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>              speedmask |= port->speedmask;
>          }
> @@ -3349,6 +3350,7 @@ static void usb_xhci_init(XHCIState *xhci)
>              }
>              port->uport = &xhci->uports[i];
>              port->speedmask = USB_SPEED_MASK_SUPER;
> +            assert(i < MAXPORTS);
>              snprintf(port->name, sizeof(port->name), "usb3 port #%d", i+1);
>              speedmask |= port->speedmask;
>          }
> 

I am confused, I upgraded Fedora 32 -> 33 and am now getting this
error back, the assertion being apparently ignored:

C compiler for the host machine: cc (gcc 10.2.1 "cc (GCC) 10.2.1
20201125 (Red Hat 10.2.1-9)")

...
                      QEMU_CFLAGS: -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2
-m32 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings
-Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv  -m32
-Wold-style-declaration -Wold-style-definition -Wtype-limits
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
-Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined
-Wimplicit-fallthrough=2 -Wno-missing-include-dirs
-Wno-shift-negative-value -Wno-psabi -fstack-protector-strong
                     QEMU_LDFLAGS: -Wl,--warn-common -Wl,-z,relro
-Wl,-z,now -m32  -m32 -fstack-protector-strong
...

[889/5130] Compiling C object libcommon.fa.p/hw_usb_hcd-xhci.c.o
FAILED: libcommon.fa.p/hw_usb_hcd-xhci.c.o
cc -Ilibcommon.fa.p -I. -I.. -I../slirp -I../slirp/src
-I../capstone/include/capstone -I../dtc/libfdt -Iqapi -Itrace -Iui
-Iui/shader -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include
-I/usr/include/libmount -I/usr/include/blkid -I/usr/include/gio-unix-2.0
-I/usr/include/pixman-1 -I/usr/include/p11-kit-1
-fdiagnostics-color=auto -pipe -Wall -Winvalid-pch -Werror -std=gnu99
-O2 -g -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -m32 -D_GNU_SOURCE
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes
-Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes
-fno-strict-aliasing -fno-common -fwrapv -m32 -Wold-style-declaration
-Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs
-Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi
-fstack-protector-strong -isystem linux-headers -isystem linux-headers
-iquote tcg/i386 -iquote . -iquote accel/tcg -iquote include -iquote
disas/libvixl -pthread -fPIC -MD -MQ libcommon.fa.p/hw_usb_hcd-xhci.c.o
-MF libcommon.fa.p/hw_usb_hcd-xhci.c.o.d -o
libcommon.fa.p/hw_usb_hcd-xhci.c.o -c ../hw/usb/hcd-xhci.c
../hw/usb/hcd-xhci.c: In function 'usb_xhci_realize':
../hw/usb/hcd-xhci.c:3309:54: error: '%d' directive output may be
truncated writing between 1 and 8 bytes into a region of size 5
[-Werror=format-truncation=]
 3309 |             snprintf(port->name, sizeof(port->name), "usb2 port
#%d", i+1);
      |                                                      ^~~~~~~~~~~~~~~
../hw/usb/hcd-xhci.c:3309:54: note: directive argument in the range [1,
89478486]
In file included from /usr/include/stdio.h:866,
                 from include/qemu/osdep.h:85,
                 from ../hw/usb/hcd-xhci.c:22:
/usr/include/bits/stdio2.h:70:10: note: '__builtin___snprintf_chk'
output between 13 and 20 bytes into a destination of size 16
   70 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL
- 1,
      |
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   71 |        __bos (__s), __fmt, __va_arg_pack ());
      |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../hw/usb/hcd-xhci.c:3323:54: error: '%d' directive output may be
truncated writing between 1 and 8 bytes into a region of size 5
[-Werror=format-truncation=]
 3323 |             snprintf(port->name, sizeof(port->name), "usb3 port
#%d", i+1);
      |                                                      ^~~~~~~~~~~~~~~
../hw/usb/hcd-xhci.c:3323:54: note: directive argument in the range [1,
89478486]
In file included from /usr/include/stdio.h:866,
                 from include/qemu/osdep.h:85,
                 from ../hw/usb/hcd-xhci.c:22:
/usr/include/bits/stdio2.h:70:10: note: '__builtin___snprintf_chk'
output between 13 and 20 bytes into a destination of size 16
   70 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL
- 1,
      |
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   71 |        __bos (__s), __fmt, __va_arg_pack ());
      |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors


Re: [Qemu-devel] [PULL 7/7] hw/usb: avoid format truncation warning when formatting port name
Posted by Daniel P. Berrangé 5 years ago
On Mon, Jan 18, 2021 at 12:31:07PM +0100, Philippe Mathieu-Daudé wrote:
> Hello,
> 
> On 5/3/19 8:59 AM, Gerd Hoffmann wrote:
> > From: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > hw/usb/hcd-xhci.c: In function ‘usb_xhci_realize’:
> > hw/usb/hcd-xhci.c:3339:66: warning: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Wformat-trunca\
> > tion=]
> >  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
> >       |                                                                  ^~
> > hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
> >  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
> >       |                                                      ^~~~~~~~~~~~~~~
> > 
> > The xhci code formats the port name into a fixed length
> > buffer which is only large enough to hold port numbers
> > upto 5 digits in decimal representation. We're never
> > going to have a port number that large, so aserting the
> > port number is sensible is sufficient to tell GCC the
> > formatted string won't be truncated.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > Message-Id: <20190412121626.19829-5-berrange@redhat.com>
> > 
> > [ kraxel: also s/int/unsigned int/ to tell gcc they can't
> >           go negative. ]
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  hw/usb/hcd-xhci.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > index d8472b4fea7f..2e9a839f2bf9 100644
> > --- a/hw/usb/hcd-xhci.c
> > +++ b/hw/usb/hcd-xhci.c
> > @@ -3306,7 +3306,7 @@ static void usb_xhci_init(XHCIState *xhci)
> >  {
> >      DeviceState *dev = DEVICE(xhci);
> >      XHCIPort *port;
> > -    int i, usbports, speedmask;
> > +    unsigned int i, usbports, speedmask;
> >  
> >      xhci->usbsts = USBSTS_HCH;
> >  
> > @@ -3336,6 +3336,7 @@ static void usb_xhci_init(XHCIState *xhci)
> >                  USB_SPEED_MASK_LOW  |
> >                  USB_SPEED_MASK_FULL |
> >                  USB_SPEED_MASK_HIGH;
> > +            assert(i < MAXPORTS);
> >              snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
> >              speedmask |= port->speedmask;
> >          }
> > @@ -3349,6 +3350,7 @@ static void usb_xhci_init(XHCIState *xhci)
> >              }
> >              port->uport = &xhci->uports[i];
> >              port->speedmask = USB_SPEED_MASK_SUPER;
> > +            assert(i < MAXPORTS);
> >              snprintf(port->name, sizeof(port->name), "usb3 port #%d", i+1);
> >              speedmask |= port->speedmask;
> >          }
> > 
> 
> I am confused, I upgraded Fedora 32 -> 33 and am now getting this
> error back, the assertion being apparently ignored:

I'm not seeing this on F33 myself, but our CI is still F32. We
should upgrade that.

What are your configure args ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [Qemu-devel] [PULL 7/7] hw/usb: avoid format truncation warning when formatting port name
Posted by Philippe Mathieu-Daudé 5 years ago
On 1/18/21 12:35 PM, Daniel P. Berrangé wrote:
> On Mon, Jan 18, 2021 at 12:31:07PM +0100, Philippe Mathieu-Daudé wrote:
>> Hello,
>>
>> On 5/3/19 8:59 AM, Gerd Hoffmann wrote:
>>> From: Daniel P. Berrangé <berrange@redhat.com>
>>>
>>> hw/usb/hcd-xhci.c: In function ‘usb_xhci_realize’:
>>> hw/usb/hcd-xhci.c:3339:66: warning: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Wformat-trunca\
>>> tion=]
>>>  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>>>       |                                                                  ^~
>>> hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
>>>  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>>>       |                                                      ^~~~~~~~~~~~~~~
>>>
>>> The xhci code formats the port name into a fixed length
>>> buffer which is only large enough to hold port numbers
>>> upto 5 digits in decimal representation. We're never
>>> going to have a port number that large, so aserting the
>>> port number is sensible is sufficient to tell GCC the
>>> formatted string won't be truncated.
>>>
>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>> Message-Id: <20190412121626.19829-5-berrange@redhat.com>
>>>
>>> [ kraxel: also s/int/unsigned int/ to tell gcc they can't
>>>           go negative. ]
>>>
>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>> ---
>>>  hw/usb/hcd-xhci.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
>>> index d8472b4fea7f..2e9a839f2bf9 100644
>>> --- a/hw/usb/hcd-xhci.c
>>> +++ b/hw/usb/hcd-xhci.c
>>> @@ -3306,7 +3306,7 @@ static void usb_xhci_init(XHCIState *xhci)
>>>  {
>>>      DeviceState *dev = DEVICE(xhci);
>>>      XHCIPort *port;
>>> -    int i, usbports, speedmask;
>>> +    unsigned int i, usbports, speedmask;
>>>  
>>>      xhci->usbsts = USBSTS_HCH;
>>>  
>>> @@ -3336,6 +3336,7 @@ static void usb_xhci_init(XHCIState *xhci)
>>>                  USB_SPEED_MASK_LOW  |
>>>                  USB_SPEED_MASK_FULL |
>>>                  USB_SPEED_MASK_HIGH;
>>> +            assert(i < MAXPORTS);
>>>              snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>>>              speedmask |= port->speedmask;
>>>          }
>>> @@ -3349,6 +3350,7 @@ static void usb_xhci_init(XHCIState *xhci)
>>>              }
>>>              port->uport = &xhci->uports[i];
>>>              port->speedmask = USB_SPEED_MASK_SUPER;
>>> +            assert(i < MAXPORTS);
>>>              snprintf(port->name, sizeof(port->name), "usb3 port #%d", i+1);
>>>              speedmask |= port->speedmask;
>>>          }
>>>
>>
>> I am confused, I upgraded Fedora 32 -> 33 and am now getting this
>> error back, the assertion being apparently ignored:
> 
> I'm not seeing this on F33 myself, but our CI is still F32. We
> should upgrade that.
> 
> What are your configure args ?

Ah sorry, this is my --extra-cflags=-m32 config directory.