[Qemu-devel] [PATCH] usb: fix unbounded stack warning for xhci_dma_write_u32s

[Qemu-devel] [PATCH] usb: fix unbounded stack warning for xhci_dma_write_u32s

[Peter Xu]

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/usb/hcd-xhci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 44b6f8c..d15918f 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -698,11 +698,13 @@ static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr,
                                        uint32_t *buf, size_t len)
 {
     int i;
-    uint32_t tmp[len / sizeof(uint32_t)];
+    uint32_t tmp[12];
+    uint32_t n = len / sizeof(uint32_t);
 
     assert((len % sizeof(uint32_t)) == 0);
+    assert(n <= ARRAY_SIZE(tmp));
 
-    for (i = 0; i < (len / sizeof(uint32_t)); i++) {
+    for (i = 0; i < n; i++) {
         tmp[i] = cpu_to_le32(buf[i]);
     }
     pci_dma_write(PCI_DEVICE(xhci), addr, tmp, len);
-- 
2.4.3

Re: [Qemu-devel] [PATCH] usb: fix unbounded stack warning for xhci_dma_write_u32s

[Gerd Hoffmann]

On Do, 2016-03-10 at 10:11 +0800, Peter Xu wrote:
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  hw/usb/hcd-xhci.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 44b6f8c..d15918f 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -698,11 +698,13 @@ static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr,
>                                         uint32_t *buf, size_t len)
>  {
>      int i;
> -    uint32_t tmp[len / sizeof(uint32_t)];
> +    uint32_t tmp[12];

Where does the 12 come from?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] usb: fix unbounded stack warning for xhci_dma_write_u32s

[Peter Xu]

On Thu, Mar 10, 2016 at 08:34:13AM +0100, Gerd Hoffmann wrote:
> On Do, 2016-03-10 at 10:11 +0800, Peter Xu wrote:
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  hw/usb/hcd-xhci.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > index 44b6f8c..d15918f 100644
> > --- a/hw/usb/hcd-xhci.c
> > +++ b/hw/usb/hcd-xhci.c
> > @@ -698,11 +698,13 @@ static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr,
> >                                         uint32_t *buf, size_t len)
> >  {
> >      int i;
> > -    uint32_t tmp[len / sizeof(uint32_t)];
> > +    uint32_t tmp[12];
> 
> Where does the 12 come from?

As mentioned in previous thread, because all the callers of
xhci_dma_write_u32s() are using const size in "len". The maximum
currently is 5 * sizeof(uint32_t) = 20 bytes. Here I choose number
bigger than 5 should work for now. To make it a little bit bigger, I
just chose 12 with no specific reason... Since 8/12/16/... seems all
works for me.

Thanks.
Peter

Re: [Qemu-devel] [PATCH] usb: fix unbounded stack warning for xhci_dma_write_u32s

[Gerd Hoffmann]

On Do, 2016-03-10 at 15:56 +0800, Peter Xu wrote:
> On Thu, Mar 10, 2016 at 08:34:13AM +0100, Gerd Hoffmann wrote:
> > On Do, 2016-03-10 at 10:11 +0800, Peter Xu wrote:
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > ---
> > >  hw/usb/hcd-xhci.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > > index 44b6f8c..d15918f 100644
> > > --- a/hw/usb/hcd-xhci.c
> > > +++ b/hw/usb/hcd-xhci.c
> > > @@ -698,11 +698,13 @@ static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr,
> > >                                         uint32_t *buf, size_t len)
> > >  {
> > >      int i;
> > > -    uint32_t tmp[len / sizeof(uint32_t)];
> > > +    uint32_t tmp[12];
> > 
> > Where does the 12 come from?
> 
> As mentioned in previous thread, because all the callers of
> xhci_dma_write_u32s() are using const size in "len". The maximum
> currently is 5 * sizeof(uint32_t) = 20 bytes

Can you note that in the commit message please?

> . Here I choose number
> bigger than 5 should work for now.

Why bigger?  5 should do just fine then, and the assert added should
make sure we'll notice if this needs an update due to code changes
elsewhere.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH] usb: fix unbounded stack warning for xhci_dma_write_u32s

[Peter Xu]

On Thu, Mar 10, 2016 at 10:21:45AM +0100, Gerd Hoffmann wrote:
> On Do, 2016-03-10 at 15:56 +0800, Peter Xu wrote:
> > As mentioned in previous thread, because all the callers of
> > xhci_dma_write_u32s() are using const size in "len". The maximum
> > currently is 5 * sizeof(uint32_t) = 20 bytes
> 
> Can you note that in the commit message please?

Sure!

> 
> > . Here I choose number
> > bigger than 5 should work for now.
> 
> Why bigger?  5 should do just fine then, and the assert added should
> make sure we'll notice if this needs an update due to code changes
> elsewhere.

Will repost with 5.

Thanks.
Peter