Re: [PULL 07/13] async: Add an optional reentrancy guard to the BH API

[Alexander Bulekov]

On 230428 1143, Thomas Huth wrote:
> From: Alexander Bulekov <alxndr@bu.edu>
> 
> Devices can pass their MemoryReentrancyGuard (from their DeviceState),
> when creating new BHes. Then, the async API will toggle the guard
> before/after calling the BH call-back. This prevents bh->mmio reentrancy
> issues.
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> Message-Id: <20230427211013.2994127-3-alxndr@bu.edu>
> [thuth: Fix "line over 90 characters" checkpatch.pl error]
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---

<snip> 
>  void aio_bh_call(QEMUBH *bh)
>  {
> +    bool last_engaged_in_io = false;
> +
> +    if (bh->reentrancy_guard) {
> +        last_engaged_in_io = bh->reentrancy_guard->engaged_in_io;
> +        if (bh->reentrancy_guard->engaged_in_io) {
> +            trace_reentrant_aio(bh->ctx, bh->name);
> +        }
> +        bh->reentrancy_guard->engaged_in_io = true;
> +    }
> +
>      bh->cb(bh->opaque);
> +
> +    if (bh->reentrancy_guard) {
> +        bh->reentrancy_guard->engaged_in_io = last_engaged_in_io;
> +    }

This causes a UAF if bh was freed in bh->cb(). 
OSS-Fuzz reported this as issue 58513.

==3433535==ERROR: AddressSanitizer: heap-use-after-free on address 
0x6060000427d0 at pc 0x565542b09347 bp 0x7fff2a4cf590 sp 0x7fff2a4cf588
READ of size 8 at 0x6060000427d0 thread T0
#0 0x565542b09346 in aio_bh_call /../util/async.c:169:19
#1 0x565542b0a2cc in aio_bh_poll /../util/async.c:200:13
#2 0x565542a6a818 in aio_dispatch /../util/aio-posix.c:421:5
#3 0x565542b1156e in aio_ctx_dispatch /../util/async.c:342:5
#4 0x7fc66e3657a8 in g_main_context_dispatch 
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x547a8) (BuildId: 
77a560369e4633278bc6e75ab0587491e11d5aac)
#5 0x565542b153f9 in glib_pollfds_poll /../util/main-loop.c:290:9
#6 0x565542b13cb3 in os_host_main_loop_wait /../util/main-loop.c:313:5
#7 0x565542b1387c in main_loop_wait /../util/main-loop.c:592:11

0x6060000427d0 is located 48 bytes inside of 56-byte region 
[0x6060000427a0,0x6060000427d8)
freed by thread T0 here:
#0 0x56553eff2192 in __interceptor_free (Id: 
ba9d8c3e3344b6323a2db18d4ab0bb9948201520)
#1 0x565542b0a32f in aio_bh_poll /../util/async.c:203:13
#2 0x565542a6ed7c in aio_poll /../util/aio-posix.c:721:17
#3 0x565542380b4d in bdrv_aio_cancel /../block/io.c:2812:13
#4 0x56554231aeda in blk_aio_cancel /../block/block-backend.c:1702:5
#5 0x56553f8fc242 in ahci_reset_port /../hw/ide/ahci.c:678:13
#6 0x56553f91d073 in handle_reg_h2d_fis /../hw/ide/ahci.c:1218:17
#7 0x56553f91a6c5 in handle_cmd /../hw/ide/ahci.c:1323:13
#8 0x56553f90fb13 in check_cmd /../hw/ide/ahci.c:595:18
#9 0x56553f944b8d in ahci_check_cmd_bh /../hw/ide/ahci.c:609:5
#10 0x565542b0929c in aio_bh_call /../util/async.c:167:5
#11 0x565542b0a2cc in aio_bh_poll /../util/async.c:200:13
#12 0x565542a6a818 in aio_dispatch /../util/aio-posix.c:421:5
#13 0x565542b1156e in aio_ctx_dispatch /../util/async.c:342:5
#14 0x7fc66e3657a8 in g_main_context_dispatch 
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x547a8)