From patchwork Wed Aug 3 14:55:40 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= X-Patchwork-Id: 9261505 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B43D66048B for ; Wed, 3 Aug 2016 15:24:01 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A4F64280DE for ; Wed, 3 Aug 2016 15:24:01 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 99CAC281DB; Wed, 3 Aug 2016 15:24:01 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 315EE280DE for ; Wed, 3 Aug 2016 15:24:01 +0000 (UTC) Received: from localhost ([::1]:35154 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUy1Y-0001eO-BL for patchwork-qemu-devel@patchwork.kernel.org; Wed, 03 Aug 2016 11:24:00 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39690) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUxc8-00019u-Qv for qemu-devel@nongnu.org; Wed, 03 Aug 2016 10:57:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUxc5-0001wE-Ks for qemu-devel@nongnu.org; Wed, 03 Aug 2016 10:57:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37472) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUxc5-0001w9-Ck for qemu-devel@nongnu.org; Wed, 03 Aug 2016 10:57:41 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E087FC0641E0 for ; Wed, 3 Aug 2016 14:57:40 +0000 (UTC) Received: from localhost (ovpn-116-73.phx2.redhat.com [10.3.116.73]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u73EvcUq023695; Wed, 3 Aug 2016 10:57:40 -0400 From: marcandre.lureau@redhat.com To: qemu-devel@nongnu.org Date: Wed, 3 Aug 2016 18:55:40 +0400 Message-Id: <20160803145541.15355-36-marcandre.lureau@redhat.com> In-Reply-To: <20160803145541.15355-1-marcandre.lureau@redhat.com> References: <20160803145541.15355-1-marcandre.lureau@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 03 Aug 2016 14:57:40 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH for-2.7 v3 35/36] ahci: fix sglist leak on retry X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: pbonzini@redhat.com, =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP From: Marc-André Lureau ahci-test /x86_64/ahci/io/dma/lba28/retry triggers the following leak: Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x7fc4b2a25e20 in malloc (/lib64/libasan.so.3+0xc6e20) #1 0x7fc4993bce58 in g_malloc (/lib64/libglib-2.0.so.0+0x4ee58) #2 0x556a187d4b34 in ahci_populate_sglist hw/ide/ahci.c:896 #3 0x556a187d8237 in ahci_dma_prepare_buf hw/ide/ahci.c:1367 #4 0x556a187b5a1a in ide_dma_cb hw/ide/core.c:844 #5 0x556a187d7eec in ahci_start_dma hw/ide/ahci.c:1333 #6 0x556a187b650b in ide_start_dma hw/ide/core.c:921 #7 0x556a187b61e6 in ide_sector_start_dma hw/ide/core.c:911 #8 0x556a187b9e26 in cmd_write_dma hw/ide/core.c:1486 #9 0x556a187bd519 in ide_exec_cmd hw/ide/core.c:2027 #10 0x556a187d71c5 in handle_reg_h2d_fis hw/ide/ahci.c:1204 #11 0x556a187d7681 in handle_cmd hw/ide/ahci.c:1254 #12 0x556a187d168a in check_cmd hw/ide/ahci.c:510 #13 0x556a187d0afc in ahci_port_write hw/ide/ahci.c:314 #14 0x556a187d105d in ahci_mem_write hw/ide/ahci.c:435 #15 0x556a1831d959 in memory_region_write_accessor /home/elmarco/src/qemu/memory.c:525 #16 0x556a1831dc35 in access_with_adjusted_size /home/elmarco/src/qemu/memory.c:591 #17 0x556a18323ce3 in memory_region_dispatch_write /home/elmarco/src/qemu/memory.c:1262 #18 0x556a1828cf67 in address_space_write_continue /home/elmarco/src/qemu/exec.c:2578 #19 0x556a1828d20b in address_space_write /home/elmarco/src/qemu/exec.c:2635 #20 0x556a1828d92b in address_space_rw /home/elmarco/src/qemu/exec.c:2737 #21 0x556a1828daf7 in cpu_physical_memory_rw /home/elmarco/src/qemu/exec.c:2746 #22 0x556a183068d3 in cpu_physical_memory_write /home/elmarco/src/qemu/include/exec/cpu-common.h:72 #23 0x556a18308194 in qtest_process_command /home/elmarco/src/qemu/qtest.c:382 #24 0x556a18309999 in qtest_process_inbuf /home/elmarco/src/qemu/qtest.c:573 #25 0x556a18309a4a in qtest_read /home/elmarco/src/qemu/qtest.c:585 #26 0x556a18598b85 in qemu_chr_be_write_impl /home/elmarco/src/qemu/qemu-char.c:387 #27 0x556a18598c52 in qemu_chr_be_write /home/elmarco/src/qemu/qemu-char.c:399 #28 0x556a185a2afa in tcp_chr_read /home/elmarco/src/qemu/qemu-char.c:2902 #29 0x556a18cbaf52 in qio_channel_fd_source_dispatch io/channel-watch.c:84 Follow John Snow recommendation: Everywhere else ncq_err is used, it is accompanied by a list cleanup except for ncq_cb, which is the case you are fixing here. Move the sglist destruction inside of ncq_err and then delete it from the other two locations to keep it tidy. Call dma_buf_commit in ide_dma_cb after the early return. Though, this is also a little wonky because this routine does more than clear the list, but it is at the moment the centralized "we're done with the sglist" function and none of the other side effects that occur in dma_buf_commit will interfere with the reset that occurs from ide_restart_bh, I think Signed-off-by: Marc-André Lureau Reviewed-by: John Snow --- hw/ide/ahci.c | 3 +-- hw/ide/core.c | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 6defeed..f3438ad 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -919,6 +919,7 @@ static void ncq_err(NCQTransferState *ncq_tfs) ide_state->error = ABRT_ERR; ide_state->status = READY_STAT | ERR_STAT; ncq_tfs->drive->port_regs.scr_err |= (1 << ncq_tfs->tag); + qemu_sglist_destroy(&ncq_tfs->sglist); ncq_tfs->used = 0; } @@ -1025,7 +1026,6 @@ static void execute_ncq_command(NCQTransferState *ncq_tfs) default: DPRINTF(port, "error: unsupported NCQ command (0x%02x) received\n", ncq_tfs->cmd); - qemu_sglist_destroy(&ncq_tfs->sglist); ncq_err(ncq_tfs); } } @@ -1092,7 +1092,6 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, error_report("ahci: PRDT length for NCQ command (0x%zx) " "is smaller than the requested size (0x%zx)", ncq_tfs->sglist.size, size); - qemu_sglist_destroy(&ncq_tfs->sglist); ncq_err(ncq_tfs); ahci_trigger_irq(ad->hba, ad, PORT_IRQ_OVERFLOW); return; diff --git a/hw/ide/core.c b/hw/ide/core.c index f9c8162..82d7f2a 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -822,6 +822,7 @@ static void ide_dma_cb(void *opaque, int ret) return; } if (ret < 0) { + dma_buf_commit(s, 0); if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) { s->bus->dma->aiocb = NULL; return;