* [PATCH] xhci: Fix memory leak in xhci_kick_epctx when poweroff GuestOS
@ 2020-01-10 10:58 kuhn.chenqun
2020-01-10 13:14 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: kuhn.chenqun @ 2020-01-10 10:58 UTC (permalink / raw)
To: qemu-devel, kraxel
Cc: qemu-trivial, Chen Qun, pannengyuan, zhang.zhanghailiang
From: Chen Qun <kuhn.chenqun@huawei.com>
start vm with libvirt, when GuestOS running, enter poweroff command using
the xhci keyboard, then ASAN shows memory leak stack:
Direct leak of 80 byte(s) in 5 object(s) allocated from:
#0 0xfffd1e6431cb in __interceptor_malloc (/lib64/libasan.so.4+0xd31cb)
#1 0xfffd1e107163 in g_malloc (/lib64/libglib-2.0.so.0+0x57163)
#2 0xaaad39051367 in qemu_sglist_init /qemu/dma-helpers.c:43
#3 0xaaad3947c407 in pci_dma_sglist_init /qemu/include/hw/pci/pci.h:842
#4 0xaaad3947c407 in xhci_xfer_create_sgl /qemu/hw/usb/hcd-xhci.c:1446
#5 0xaaad3947c407 in xhci_setup_packet /qemu/hw/usb/hcd-xhci.c:1618
#6 0xaaad3948625f in xhci_submit /qemu/hw/usb/hcd-xhci.c:1827
#7 0xaaad3948625f in xhci_fire_transfer /qemu/hw/usb/hcd-xhci.c:1839
#8 0xaaad3948625f in xhci_kick_epctx /qemu/hw/usb/hcd-xhci.c:1991
#9 0xaaad3948f537 in xhci_doorbell_write /qemu/hw/usb/hcd-xhci.c:3158
#10 0xaaad38bcbfc7 in memory_region_write_accessor /qemu/memory.c:483
#11 0xaaad38bc654f in access_with_adjusted_size /qemu/memory.c:544
#12 0xaaad38bd1877 in memory_region_dispatch_write /qemu/memory.c:1482
#13 0xaaad38b1c77f in flatview_write_continue /qemu/exec.c:3167
#14 0xaaad38b1ca83 in flatview_write /qemu/exec.c:3207
#15 0xaaad38b268db in address_space_write /qemu/exec.c:3297
#16 0xaaad38bf909b in kvm_cpu_exec /qemu/accel/kvm/kvm-all.c:2383
#17 0xaaad38bb063f in qemu_kvm_cpu_thread_fn /qemu/cpus.c:1246
#18 0xaaad39821c93 in qemu_thread_start /qemu/util/qemu-thread-posix.c:519
#19 0xfffd1c8378bb (/lib64/libpthread.so.0+0x78bb)
#20 0xfffd1c77616b (/lib64/libc.so.6+0xd616b)
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
---
hw/usb/hcd-xhci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 80988bb305..0d3d96d05a 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2000,6 +2000,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
if (xfer != NULL && xfer->running_retry) {
DPRINTF("xhci: xfer nacked, stopping schedule\n");
epctx->retry = xfer;
+ xhci_xfer_unmap(xfer);
break;
}
if (count++ > TRANSFER_LIMIT) {
--
2.23.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] xhci: Fix memory leak in xhci_kick_epctx when poweroff GuestOS
2020-01-10 10:58 [PATCH] xhci: Fix memory leak in xhci_kick_epctx when poweroff GuestOS kuhn.chenqun
@ 2020-01-10 13:14 ` Philippe Mathieu-Daudé
2020-01-11 2:15 ` Chenqun (kuhn)
2020-01-13 7:48 ` Gerd Hoffmann
0 siblings, 2 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-10 13:14 UTC (permalink / raw)
To: kuhn.chenqun, qemu-devel, kraxel
Cc: qemu-trivial, pannengyuan, zhang.zhanghailiang
On 1/10/20 11:58 AM, kuhn.chenqun@huawei.com wrote:
> From: Chen Qun <kuhn.chenqun@huawei.com>
>
> start vm with libvirt, when GuestOS running, enter poweroff command using
> the xhci keyboard, then ASAN shows memory leak stack:
>
> Direct leak of 80 byte(s) in 5 object(s) allocated from:
> #0 0xfffd1e6431cb in __interceptor_malloc (/lib64/libasan.so.4+0xd31cb)
> #1 0xfffd1e107163 in g_malloc (/lib64/libglib-2.0.so.0+0x57163)
> #2 0xaaad39051367 in qemu_sglist_init /qemu/dma-helpers.c:43
> #3 0xaaad3947c407 in pci_dma_sglist_init /qemu/include/hw/pci/pci.h:842
> #4 0xaaad3947c407 in xhci_xfer_create_sgl /qemu/hw/usb/hcd-xhci.c:1446
> #5 0xaaad3947c407 in xhci_setup_packet /qemu/hw/usb/hcd-xhci.c:1618
> #6 0xaaad3948625f in xhci_submit /qemu/hw/usb/hcd-xhci.c:1827
> #7 0xaaad3948625f in xhci_fire_transfer /qemu/hw/usb/hcd-xhci.c:1839
> #8 0xaaad3948625f in xhci_kick_epctx /qemu/hw/usb/hcd-xhci.c:1991
> #9 0xaaad3948f537 in xhci_doorbell_write /qemu/hw/usb/hcd-xhci.c:3158
> #10 0xaaad38bcbfc7 in memory_region_write_accessor /qemu/memory.c:483
> #11 0xaaad38bc654f in access_with_adjusted_size /qemu/memory.c:544
> #12 0xaaad38bd1877 in memory_region_dispatch_write /qemu/memory.c:1482
> #13 0xaaad38b1c77f in flatview_write_continue /qemu/exec.c:3167
> #14 0xaaad38b1ca83 in flatview_write /qemu/exec.c:3207
> #15 0xaaad38b268db in address_space_write /qemu/exec.c:3297
> #16 0xaaad38bf909b in kvm_cpu_exec /qemu/accel/kvm/kvm-all.c:2383
> #17 0xaaad38bb063f in qemu_kvm_cpu_thread_fn /qemu/cpus.c:1246
> #18 0xaaad39821c93 in qemu_thread_start /qemu/util/qemu-thread-posix.c:519
> #19 0xfffd1c8378bb (/lib64/libpthread.so.0+0x78bb)
> #20 0xfffd1c77616b (/lib64/libc.so.6+0xd616b)
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> ---
> hw/usb/hcd-xhci.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 80988bb305..0d3d96d05a 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -2000,6 +2000,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
> if (xfer != NULL && xfer->running_retry) {
> DPRINTF("xhci: xfer nacked, stopping schedule\n");
> epctx->retry = xfer;
> + xhci_xfer_unmap(xfer);
Shouldn't we use xhci_ep_free_xfer() instead?
Also, it would be cleaner if you set it to NULL.
> break;
> }
> if (count++ > TRANSFER_LIMIT) {
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] xhci: Fix memory leak in xhci_kick_epctx when poweroff GuestOS
2020-01-10 13:14 ` Philippe Mathieu-Daudé
@ 2020-01-11 2:15 ` Chenqun (kuhn)
2020-01-13 7:48 ` Gerd Hoffmann
1 sibling, 0 replies; 5+ messages in thread
From: Chenqun (kuhn) @ 2020-01-11 2:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, kraxel
Cc: qemu-trivial, Pannengyuan, Zhanghailiang
>-----Original Message-----
>From: Philippe Mathieu-Daudé [mailto:philmd@redhat.com]
>Sent: Friday, January 10, 2020 9:14 PM
>To: Chenqun (kuhn) <kuhn.chenqun@huawei.com>; qemu-
>devel@nongnu.org; kraxel@redhat.com
>Cc: qemu-trivial@nongnu.org; Pannengyuan <pannengyuan@huawei.com>;
>Zhanghailiang <zhang.zhanghailiang@huawei.com>
>Subject: Re: [PATCH] xhci: Fix memory leak in xhci_kick_epctx when poweroff
>GuestOS
>
>On 1/10/20 11:58 AM, kuhn.chenqun@huawei.com wrote:
>> From: Chen Qun <kuhn.chenqun@huawei.com>
>>
>> start vm with libvirt, when GuestOS running, enter poweroff command
>> using the xhci keyboard, then ASAN shows memory leak stack:
>>
>> Direct leak of 80 byte(s) in 5 object(s) allocated from:
>> #0 0xfffd1e6431cb in __interceptor_malloc (/lib64/libasan.so.4+0xd31cb)
>> #1 0xfffd1e107163 in g_malloc (/lib64/libglib-2.0.so.0+0x57163)
>> #2 0xaaad39051367 in qemu_sglist_init /qemu/dma-helpers.c:43
>> #3 0xaaad3947c407 in pci_dma_sglist_init
>/qemu/include/hw/pci/pci.h:842
>> #4 0xaaad3947c407 in xhci_xfer_create_sgl /qemu/hw/usb/hcd-
>xhci.c:1446
>> #5 0xaaad3947c407 in xhci_setup_packet /qemu/hw/usb/hcd-xhci.c:1618
>> #6 0xaaad3948625f in xhci_submit /qemu/hw/usb/hcd-xhci.c:1827
>> #7 0xaaad3948625f in xhci_fire_transfer /qemu/hw/usb/hcd-xhci.c:1839
>> #8 0xaaad3948625f in xhci_kick_epctx /qemu/hw/usb/hcd-xhci.c:1991
>> #9 0xaaad3948f537 in xhci_doorbell_write /qemu/hw/usb/hcd-
>xhci.c:3158
>> #10 0xaaad38bcbfc7 in memory_region_write_accessor
>/qemu/memory.c:483
>> #11 0xaaad38bc654f in access_with_adjusted_size /qemu/memory.c:544
>> #12 0xaaad38bd1877 in memory_region_dispatch_write
>/qemu/memory.c:1482
>> #13 0xaaad38b1c77f in flatview_write_continue /qemu/exec.c:3167
>> #14 0xaaad38b1ca83 in flatview_write /qemu/exec.c:3207
>> #15 0xaaad38b268db in address_space_write /qemu/exec.c:3297
>> #16 0xaaad38bf909b in kvm_cpu_exec /qemu/accel/kvm/kvm-all.c:2383
>> #17 0xaaad38bb063f in qemu_kvm_cpu_thread_fn /qemu/cpus.c:1246
>> #18 0xaaad39821c93 in qemu_thread_start /qemu/util/qemu-thread-
>posix.c:519
>> #19 0xfffd1c8378bb (/lib64/libpthread.so.0+0x78bb)
>> #20 0xfffd1c77616b (/lib64/libc.so.6+0xd616b)
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
>> ---
>> hw/usb/hcd-xhci.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index
>> 80988bb305..0d3d96d05a 100644
>> --- a/hw/usb/hcd-xhci.c
>> +++ b/hw/usb/hcd-xhci.c
>> @@ -2000,6 +2000,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx,
>unsigned int streamid)
>> if (xfer != NULL && xfer->running_retry) {
>> DPRINTF("xhci: xfer nacked, stopping schedule\n");
>> epctx->retry = xfer;
>> + xhci_xfer_unmap(xfer);
>
>Shouldn't we use xhci_ep_free_xfer() instead?
>Also, it would be cleaner if you set it to NULL.
>
Hi Philippe,
Thanks for your reply!
But, It is just a sglist leak. xhci_xfer_unmap() can free and set it to NULL.
The xhci_ep_free_xfer() did't free a sglist.
By the way, xfer should be use for epctx->retry, we can't free it.
Thanks.
>> break;
>> }
>> if (count++ > TRANSFER_LIMIT) {
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xhci: Fix memory leak in xhci_kick_epctx when poweroff GuestOS
2020-01-10 13:14 ` Philippe Mathieu-Daudé
2020-01-11 2:15 ` Chenqun (kuhn)
@ 2020-01-13 7:48 ` Gerd Hoffmann
2020-01-13 9:39 ` Chenqun (kuhn)
1 sibling, 1 reply; 5+ messages in thread
From: Gerd Hoffmann @ 2020-01-13 7:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-trivial, kuhn.chenqun, pannengyuan, qemu-devel, zhang.zhanghailiang
> > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > index 80988bb305..0d3d96d05a 100644
> > --- a/hw/usb/hcd-xhci.c
> > +++ b/hw/usb/hcd-xhci.c
> > @@ -2000,6 +2000,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
> > if (xfer != NULL && xfer->running_retry) {
> > DPRINTF("xhci: xfer nacked, stopping schedule\n");
> > epctx->retry = xfer;
> > + xhci_xfer_unmap(xfer);
>
> Shouldn't we use xhci_ep_free_xfer() instead?
No, xhci will try to run the transfer again later.
xhci will re-create the sgl then, so freeing the
sgl here is correct. Patch added to usb queue.
thanks,
Gerd
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] xhci: Fix memory leak in xhci_kick_epctx when poweroff GuestOS
2020-01-13 7:48 ` Gerd Hoffmann
@ 2020-01-13 9:39 ` Chenqun (kuhn)
0 siblings, 0 replies; 5+ messages in thread
From: Chenqun (kuhn) @ 2020-01-13 9:39 UTC (permalink / raw)
To: Gerd Hoffmann, Philippe Mathieu-Daudé
Cc: qemu-trivial, Pannengyuan, qemu-devel, Zhanghailiang
>-----Original Message-----
>From: Gerd Hoffmann [mailto:kraxel@redhat.com]
>Sent: Monday, January 13, 2020 3:48 PM
>To: Philippe Mathieu-Daudé <philmd@redhat.com>
>Cc: Chenqun (kuhn) <kuhn.chenqun@huawei.com>; qemu-
>devel@nongnu.org; qemu-trivial@nongnu.org; Pannengyuan
><pannengyuan@huawei.com>; Zhanghailiang
><zhang.zhanghailiang@huawei.com>
>Subject: Re: [PATCH] xhci: Fix memory leak in xhci_kick_epctx when poweroff
>GuestOS
>
>> > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index
>> > 80988bb305..0d3d96d05a 100644
>> > --- a/hw/usb/hcd-xhci.c
>> > +++ b/hw/usb/hcd-xhci.c
>> > @@ -2000,6 +2000,7 @@ static void xhci_kick_epctx(XHCIEPContext
>*epctx, unsigned int streamid)
>> > if (xfer != NULL && xfer->running_retry) {
>> > DPRINTF("xhci: xfer nacked, stopping schedule\n");
>> > epctx->retry = xfer;
>> > + xhci_xfer_unmap(xfer);
>>
>> Shouldn't we use xhci_ep_free_xfer() instead?
>
>No, xhci will try to run the transfer again later.
>
>xhci will re-create the sgl then, so freeing the sgl here is correct. Patch added
>to usb queue.
Hi Gerd,
I test every keyboard input, it will leak once.
I tested qemu-4.0.0 also had this leak .
Maybe we should cc to qemu-stable ?
Thanks.
>
>thanks,
> Gerd
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-01-13 9:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-10 10:58 [PATCH] xhci: Fix memory leak in xhci_kick_epctx when poweroff GuestOS kuhn.chenqun
2020-01-10 13:14 ` Philippe Mathieu-Daudé
2020-01-11 2:15 ` Chenqun (kuhn)
2020-01-13 7:48 ` Gerd Hoffmann
2020-01-13 9:39 ` Chenqun (kuhn)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.