[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] ahci: Fix sglist memleak in ahci_dma_rw_buf
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] ahci: Fix sglist memleak in ahci_dma_rw_buf() |
Date: |
Fri, 27 Jul 2012 09:16:05 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120615 Thunderbird/13.0.1 |
Il 26/07/2012 21:40, Jason Baron ha scritto:
> I noticed that in hw/ide/ahci:ahci_dma_rw_buf() does not appear to free the
> sglist. Thus, I've added a call to qemu_sglist_destroy() to fix this memory
> leak.
>
> I'm wondering though if 'ahci_populate_sglist()' can return 0, and not
> populate the sglist, thus causing us to call free on NULL pointer. However, I
> see that ahci_start_transfer() always calls the free if the return is 0.
A free(NULL) is ok, but a double-free would not be. Something like this
would make me feel better:
diff --git a/dma-helpers.c b/dma-helpers.c
index 35cb500..57725d0 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -65,6 +65,7 @@
void qemu_sglist_destroy(QEMUSGList *qsg)
{
g_free(qsg->sg);
+ memset(qsg, 0, sizeof(qsg));
}
typedef struct {
Paolo
> Signed-off-by: Jason Baron <address@hidden>
> ---
> hw/ide/ahci.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 9c95714..b48401d 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1073,6 +1073,9 @@ static int ahci_dma_rw_buf(IDEDMA *dma, int is_write)
> dma_buf_write(p, l, &s->sg);
> }
>
> + /* free sglist that was created in ahci_populate_sglist() */
> + qemu_sglist_destroy(&s->sg);
> +
> /* update number of transferred bytes */
> ad->cur_cmd->status = cpu_to_le32(le32_to_cpu(ad->cur_cmd->status) + l);
> s->io_buffer_index += l;
>