[Qemu-devel] [PATCH for-2.8] megasas: do not call pci_dma_unmap after having freed the frame once

[Qemu-devel] [PATCH for-2.8] megasas: do not call pci_dma_unmap after having freed the frame once

[Paolo Bonzini]

Commit 8cc4678 ("megasas: remove useless check for cmd->frame", 2016-07-17) was
wrong because I trusted Coverity too much.  It turns out that there _is_ a
path through which cmd->frame can become NULL.  After megasas_handle_frame's
switch (md->frame->header.frame_cmd), megasas_init_firmware can be called.
>From there, megasas_reset_frames will call megasas_unmap_frame which resets
cmd->frame = NULL.

However, there is another bug to fix in there, because megasas_unmap_frame
is called again after setting the command status.  In this case QEMU should
not do anything, instead it calls pci_dma_unmap again.  Harmless, but
better fix it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/megasas.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 52a4123..ca62952 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -461,9 +461,12 @@ static void megasas_unmap_frame(MegasasState *s, MegasasCmd *cmd)
 {
     PCIDevice *p = PCI_DEVICE(s);
 
-    pci_dma_unmap(p, cmd->frame, cmd->pa_size, 0, 0);
+    if (cmd->pa_size) {
+        pci_dma_unmap(p, cmd->frame, cmd->pa_size, 0, 0);
+    }
     cmd->frame = NULL;
     cmd->pa = 0;
+    cmd->pa_size = 0;
     clear_bit(cmd->index, s->frame_map);
 }
 
-- 
2.9.3

Re: [Qemu-devel] [PATCH for-2.8] megasas: do not call pci_dma_unmap after having freed the frame once

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

On Thu, Nov 10, 2016 at 04:27:50PM +0100, Paolo Bonzini wrote:
> Commit 8cc4678 ("megasas: remove useless check for cmd->frame", 2016-07-17) was
> wrong because I trusted Coverity too much.  It turns out that there _is_ a
> path through which cmd->frame can become NULL.  After megasas_handle_frame's
> switch (md->frame->header.frame_cmd), megasas_init_firmware can be called.
> From there, megasas_reset_frames will call megasas_unmap_frame which resets
> cmd->frame = NULL.
> 
> However, there is another bug to fix in there, because megasas_unmap_frame
> is called again after setting the command status.  In this case QEMU should
> not do anything, instead it calls pci_dma_unmap again.  Harmless, but
> better fix it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/scsi/megasas.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]