Skip to content

Commit 3d34297

Browse files
elmarcomdroth
authored andcommitted
ahci: fix sglist leak on retry
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 <[email protected]> Reviewed-by: John Snow <[email protected]> (cherry picked from commit 5839df7) Signed-off-by: Michael Roth <[email protected]>
1 parent ff71767 commit 3d34297

File tree

2 files changed

+2
-2
lines changed

2 files changed

+2
-2
lines changed

hw/ide/ahci.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -919,6 +919,7 @@ static void ncq_err(NCQTransferState *ncq_tfs)
919919
ide_state->error = ABRT_ERR;
920920
ide_state->status = READY_STAT | ERR_STAT;
921921
ncq_tfs->drive->port_regs.scr_err |= (1 << ncq_tfs->tag);
922+
qemu_sglist_destroy(&ncq_tfs->sglist);
922923
ncq_tfs->used = 0;
923924
}
924925

@@ -1023,7 +1024,6 @@ static void execute_ncq_command(NCQTransferState *ncq_tfs)
10231024
default:
10241025
DPRINTF(port, "error: unsupported NCQ command (0x%02x) received\n",
10251026
ncq_tfs->cmd);
1026-
qemu_sglist_destroy(&ncq_tfs->sglist);
10271027
ncq_err(ncq_tfs);
10281028
}
10291029
}
@@ -1090,7 +1090,6 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis,
10901090
error_report("ahci: PRDT length for NCQ command (0x%zx) "
10911091
"is smaller than the requested size (0x%zx)",
10921092
ncq_tfs->sglist.size, size);
1093-
qemu_sglist_destroy(&ncq_tfs->sglist);
10941093
ncq_err(ncq_tfs);
10951094
ahci_trigger_irq(ad->hba, ad, PORT_IRQ_OVERFLOW);
10961095
return;

hw/ide/core.c

+1
Original file line numberDiff line numberDiff line change
@@ -807,6 +807,7 @@ static void ide_dma_cb(void *opaque, int ret)
807807
if (ret < 0) {
808808
if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
809809
s->bus->dma->aiocb = NULL;
810+
dma_buf_commit(s, 0);
810811
return;
811812
}
812813
}

0 commit comments

Comments
 (0)