Re: [Qemu-devel] AIO error case

[John Snow <jsnow@redhat.com>]

On 05/23/2018 02:25 PM, Nishanth Aravamudan wrote:
> On Wed, May 23, 2018 at 10:53 AM, John Snow <jsnow@redhat.com
> <mailto:jsnow@redhat.com>> wrote:
>>
>>
>>
>> On 05/22/2018 06:01 PM, Nishanth Aravamudan via Qemu-devel wrote:
>> > Hi!
>> >
>>
>> Hi! CCing qemu-block@nongnu.org <mailto:qemu-block@nongnu.org>;
>>
>> > I'm tracking an error case in the native AIO path, and was wondering if
>> > there was a latent (albeit possibly hard to hit) bug. Specifically
>> > util/async.c::aio_get_linux_aio:
>> >
>> > #ifdef CONFIG_LINUX_AIO
>> > LinuxAioState *aio_get_linux_aio(AioContext *ctx)
>> > {
>> >     if (!ctx->linux_aio) {
>> >         ctx->linux_aio = laio_init();
>> >         laio_attach_aio_context(ctx->linux_aio, ctx);
>> >     }
>> >     return ctx->linux_aio;
>> > }
>> > #endif
>> >
>> > laio_init() can in certain conditions return NULL, but that's not
> checked
>> > here and then the NULL result is passed directly into
>> > laio_attach_aio_context, which dereferences it without checking that the
>> > pointer is valid.
>> >
>>
>> Looks like a good old-fashioned bug to me:
> 
> 
> Agreed!
>  
> <snip>
> 
>> Wanna send a patch?
> 
> Yep I'll work on this over the next few days. Thanks for reply!
> 
> -Nish

I looked at plug and unplug and it really looks like -- apart from the
memoization of aio_get_linux_aio that might fail -- there's nothing in
those calls that is expected to actually break.

Might be saner to try to force the memoization earlier for virtio-blk
and virtio-scsi and test the return at that time; then just assert that
aio_get_linux_aio actually returns non-null in calls like un/plug that
cannot fail.

Should save you a lot of rewiring work.

--js