Re: [Qemu-devel] [PATCH] macio: fix NULL pointer dereference when issuing IDE trim

[John Snow <jsnow@redhat.com>]

On 02/26/2018 03:56 AM, Anton Nefedov wrote:
> 
> 
> On 23/2/2018 9:47 PM, Mark Cave-Ayland wrote:
>> Commit ef0e64a983 "ide: pass IDEState to trim AIO callback" changed the
>> IDE trim callback from using a BlockBackend to an IDEState but forgot
>> to update
>> the dma_blk_io() call in hw/ide/macio.c accordingly.
>>
> 
> I somehow missed this whole macio part in that series :(
> 

It's my mistake entirely.

>> Without this fix qemu-system-ppc segfaults when issuing an IDE trim
>> command on
>> any of the PPC Mac machines (easily triggered by running the Debian
>> installer).
>>
>> Reported-by: Howard Spoelstra <hsp.cat7@gmail.com>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> Reviewed-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> 
> ..but there should also be a fix-up for
> 947858b "ide: abort TRIM operation for invalid range"
> which apparently lacks a few steps on the invalid range errorpath for
> macio. I'll look into that.
> 

I'm unfortunately a little preoccupied right now, please CC me
(hopefully before 2.12 freeze!) and I'll get this squared away for next
release.

>> ---
>>   hw/ide/macio.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
>> index 2e043ef1ea..d3a85cba3b 100644
>> --- a/hw/ide/macio.c
>> +++ b/hw/ide/macio.c
>> @@ -187,7 +187,7 @@ static void pmac_ide_transfer_cb(void *opaque, int
>> ret)
>>           break;
>>       case IDE_DMA_TRIM:
>>           s->bus->dma->aiocb = dma_blk_io(blk_get_aio_context(s->blk),
>> &s->sg,
>> -                                        offset, 0x1, ide_issue_trim,
>> s->blk,
>> +                                        offset, 0x1, ide_issue_trim, s,
>>                                           pmac_ide_transfer_cb, io,
>>                                           DMA_DIRECTION_TO_DEVICE);
>>           break;
>>
> 

In the meantime, I'm going to stage this for tomorrow so Mark doesn't
have to deal with a broken tree.

--js