[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s |
Date: |
Wed, 9 Mar 2016 13:12:21 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Tue, Mar 08, 2016 at 02:26:36PM +0700, Peter Maydell wrote:
> On 8 March 2016 at 14:00, Peter Xu <address@hidden> wrote:
> > First of all, this function cannot be inlined even with always_inline,
> > so removing inline.
>
> Please don't mix two different changes in one patch.
Sorry. Will follow this.
>
> > After that, make its stack bounded.
> >
> > Suggested-by: Paolo Bonzini <address@hidden>
> > CC: Gerd Hoffmann <address@hidden>
> > Signed-off-by: Peter Xu <address@hidden>
> > ---
> > hw/usb/hcd-xhci.c | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > index 44b6f8c..3dd5a02 100644
> > --- a/hw/usb/hcd-xhci.c
> > +++ b/hw/usb/hcd-xhci.c
> > @@ -694,18 +694,22 @@ static inline void xhci_dma_read_u32s(XHCIState
> > *xhci, dma_addr_t addr,
> > }
> > }
> >
> > -static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr,
> > - uint32_t *buf, size_t len)
> > +static 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 n = len / sizeof(uint32_t);
> > +#define __BUF_SIZE (12)
> > + uint32_t tmp[__BUF_SIZE];
> >
> > + assert(__BUF_SIZE >= n);
> > assert((len % sizeof(uint32_t)) == 0);
> >
> > - 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);
> > +#undef __BUF_SIZE
>
> All the patches in this series seem to be following the
> same pattern of #defining an arbitrary fixed length for
> the array. This does not at all seem to me to be an
> improvement. We should be avoiding unbounded stack
> allocations, but we need to do this by either changing
> the code to work correctly without an unbounded allocation
> or by using a heap allocation instead of a stack allocation.
>
> In some cases, like this one, the original code isn't even
> unbounded -- we always call this function with a length
> parameter which is a small compile time constant, so the
> stack usage is definitely bounded. So this change is making
> the code uglier and less flexible for no benefit that I
> can see.
I was trying to avoid the "stack unlimited" warning. There will be a
warning generated before applying this patch. The patch solved
this. Though I'd say this patch might not be good enough. :(
Then, will drop this one too if I have no further better
idea.
Thanks for review.
Peter
[Qemu-devel] [PATCH 3/8] usb: fix unbounded stack for ohci_td_pkt, Peter Xu, 2016/03/08
[Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s, Peter Xu, 2016/03/08
- Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s, Peter Maydell, 2016/03/08
- Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s,
Peter Xu <=
- Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s, Paolo Bonzini, 2016/03/08
- Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s, Peter Xu, 2016/03/09
- Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s, Paolo Bonzini, 2016/03/09
- Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s, Peter Xu, 2016/03/09
- Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s, Markus Armbruster, 2016/03/09
- Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s, Peter Xu, 2016/03/09
- Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s, Markus Armbruster, 2016/03/09
Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s, Paolo Bonzini, 2016/03/09
Re: [Qemu-devel] [PATCH 4/8] usb: fix unbounded stack for xhci_dma_write_u32s, Peter Xu, 2016/03/09
[Qemu-devel] [PATCH 5/8] usb: fix unbounded stack for inotify_watchfn, Peter Xu, 2016/03/08