diff mbox series

block/monitor/block-hmp-cmds.c: Fix crash when execute hmp_commit

Message ID 20230423110254.35672-1-wangliangzz@126.com
State New
Headers show
Series block/monitor/block-hmp-cmds.c: Fix crash when execute hmp_commit | expand

Commit Message

Wang Liang April 23, 2023, 11:02 a.m. UTC
From: Wang Liang <wangliangzz@inspur.com>

We need to get the aio_context before calling the blk_is_available.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1615
Signed-off-by: Wang Liang <wangliangzz@inspur.com>
---
 block/monitor/block-hmp-cmds.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Emanuele Giuseppe Esposito April 24, 2023, 8:15 a.m. UTC | #1
Am 23/04/2023 um 13:02 schrieb wangliangzz@126.com:
> From: Wang Liang <wangliangzz@inspur.com>
>
> We need to get the aio_context before calling the blk_is_available.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1615
> Signed-off-by: Wang Liang <wangliangzz@inspur.com>
>


Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Emanuele Giuseppe Esposito April 24, 2023, 8:36 a.m. UTC | #2
Am 24/04/2023 um 10:15 schrieb Emanuele Giuseppe Esposito:
> 
> 
> Am 23/04/2023 um 13:02 schrieb wangliangzz@126.com:
>> From: Wang Liang <wangliangzz@inspur.com>
>>
>> We need to get the aio_context before calling the blk_is_available.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1615
>> Signed-off-by: Wang Liang <wangliangzz@inspur.com>
>>
> 
> 
> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> 

Sorry I forgot, if you need to resend can you add the cause of this
issue in the commit message?
Something along the lines of:
"hmp_commit() calls blk_is_available() from a non-coroutine context (and
in the main loop). Since this is a co_wrapper_mixed_bdrv_rdlock
function, in this case it calls AIO_WAIT_WHILE(), which crashes if the
aio_context lock is not taken before"

Thank you,
Emanuele
Wang Liang April 24, 2023, 9:25 a.m. UTC | #3
On Mon, 2023-04-24 at 10:36 +0200, Emanuele Giuseppe Esposito wrote:
> 
> Am 24/04/2023 um 10:15 schrieb Emanuele Giuseppe Esposito:
> > 
> > Am 23/04/2023 um 13:02 schrieb wangliangzz@126.com:
> > > From: Wang Liang <wangliangzz@inspur.com>
> > > 
> > > We need to get the aio_context before calling the
> > > blk_is_available.
> > > 
> > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1615
> > > Signed-off-by: Wang Liang <wangliangzz@inspur.com>
> > > 
> > 
> > Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> > 
> 
> Sorry I forgot, if you need to resend can you add the cause of this
> issue in the commit message?
> Something along the lines of:
> "hmp_commit() calls blk_is_available() from a non-coroutine context
> (and
> in the main loop). Since this is a co_wrapper_mixed_bdrv_rdlock
> function, in this case it calls AIO_WAIT_WHILE(), which crashes if
> the
> aio_context lock is not taken before"
> 
> Thank you,
> Emanuele

Thanks for the detailed explanation. I'll resend the patch.

Wang Liang
diff mbox series

Patch

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 2846083546..ca2599de44 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -214,15 +214,17 @@  void hmp_commit(Monitor *mon, const QDict *qdict)
             error_report("Device '%s' not found", device);
             return;
         }
-        if (!blk_is_available(blk)) {
-            error_report("Device '%s' has no medium", device);
-            return;
-        }
 
         bs = bdrv_skip_implicit_filters(blk_bs(blk));
         aio_context = bdrv_get_aio_context(bs);
         aio_context_acquire(aio_context);
 
+        if (!blk_is_available(blk)) {
+            error_report("Device '%s' has no medium", device);
+            aio_context_release(aio_context);
+            return;
+        }
+
         ret = bdrv_commit(bs);
 
         aio_context_release(aio_context);