diff mbox

usb: fix unbounded stack warning for xhci_dma_write_u32s

Message ID 1457575914-15581-1-git-send-email-peterx@redhat.com
State New
Headers show

Commit Message

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

Comments

Gerd Hoffmann March 10, 2016, 7:34 a.m. UTC | #1
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
Peter Xu March 10, 2016, 7:56 a.m. UTC | #2
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
Gerd Hoffmann March 10, 2016, 9:21 a.m. UTC | #3
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
Peter Xu March 11, 2016, 1:44 a.m. UTC | #4
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
diff mbox

Patch

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);