Re: [Qemu-devel] [PATCH 3/8] usb: fix unbounded stack for ohci_td_pkt

[Paolo Bonzini <pbonzini@redhat.com>]

On 08/03/2016 08:00, Peter Xu wrote:
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> CC: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  hw/usb/hcd-ohci.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index 17ed461..c3cd4e2 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -936,11 +936,11 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
>  #ifdef trace_event_get_state
>  static void ohci_td_pkt(const char *msg, const uint8_t *buf, size_t len)
>  {
> +#define __TEMP_WIDTH (16)
>      bool print16 = !!trace_event_get_state(TRACE_USB_OHCI_TD_PKT_SHORT);
>      bool printall = !!trace_event_get_state(TRACE_USB_OHCI_TD_PKT_FULL);
> -    const int width = 16;
>      int i;
> -    char tmp[3 * width + 1];
> +    char tmp[3 * __TEMP_WIDTH + 1];
>      char *p = tmp;
>  
>      if (!printall && !print16) {
> @@ -948,7 +948,7 @@ static void ohci_td_pkt(const char *msg, const uint8_t *buf, size_t len)
>      }
>  
>      for (i = 0; ; i++) {
> -        if (i && (!(i % width) || (i == len))) {
> +        if (i && (!(i % __TEMP_WIDTH) || (i == len))) {
>              if (!printall) {
>                  trace_usb_ohci_td_pkt_short(msg, tmp);
>                  break;
> @@ -963,6 +963,7 @@ static void ohci_td_pkt(const char *msg, const uint8_t *buf, size_t len)
>  
>          p += sprintf(p, " %.2x", buf[i]);
>      }
> +#undef __TEMP_WIDTH
>  }
>  #else
>  static void ohci_td_pkt(const char *msg, const uint8_t *buf, size_t len)
> 

This is a compiler false positive.  You can change "i % width" to

   p - tmp == ARRAY_SIZE(tmp) - 1

if you want to avoid it, but I'd just ignore this one.

Paolo