[Qemu-devel] [PATCH 0/1] ahci: fix ncq aiocb-related segfault

[Qemu-devel] [PATCH 0/1] ahci: fix ncq aiocb-related segfault

[John Snow]

Fix ncq_cb to prevent a segfault on sys_reset.

John Snow (1):
  ahci: clear aiocb in ncq_cb

 hw/ide/ahci.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.7.4

[Qemu-devel] [PATCH 1/1] ahci: clear aiocb in ncq_cb

[John Snow]

Similar to existing fixes for IDE and ATAPI, the AIOCB
must be cleared in the callback. Otherwise, we may accidentally
try to reset it after the object has been released.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/ahci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index f3438ad..63ead21 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -948,6 +948,7 @@ static void ncq_cb(void *opaque, int ret)
     NCQTransferState *ncq_tfs = (NCQTransferState *)opaque;
     IDEState *ide_state = &ncq_tfs->drive->port.ifs[0];
 
+    ncq_tfs->aiocb = NULL;
     if (ret == -ECANCELED) {
         return;
     }
-- 
2.7.4

Re: [Qemu-devel] [Qemu-block] [PATCH 0/1] ahci: fix ncq aiocb-related segfault

[Stefan Hajnoczi]

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

On Thu, Sep 22, 2016 at 04:10:39PM -0400, John Snow wrote:
> Fix ncq_cb to prevent a segfault on sys_reset.
> 
> John Snow (1):
>   ahci: clear aiocb in ncq_cb
> 
>  hw/ide/ahci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> -- 
> 2.7.4

Maybe worth adding as a clarification:

The issue is when bdrv_aio_cancel() is called after ncq_cb() was already
invoked.  The aiocb will be a dangling pointer.

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

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

Re: [Qemu-devel] [Qemu-block] [PATCH 0/1] ahci: fix ncq aiocb-related segfault

[John Snow]

On 09/26/2016 12:10 PM, Stefan Hajnoczi wrote:
> On Thu, Sep 22, 2016 at 04:10:39PM -0400, John Snow wrote:
>> Fix ncq_cb to prevent a segfault on sys_reset.
>>
>> John Snow (1):
>>   ahci: clear aiocb in ncq_cb
>>
>>  hw/ide/ahci.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> --
>> 2.7.4
>
> Maybe worth adding as a clarification:
>
> The issue is when bdrv_aio_cancel() is called after ncq_cb() was already
> invoked.  The aiocb will be a dangling pointer.
>

Done.

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

Thanks, applied to my IDE tree:

https://github.com/jnsnow/qemu/commits/ide
https://github.com/jnsnow/qemu.git

--js