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

wangliangzz@126.com posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230423110254.35672-1-wangliangzz@126.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
There is a newer version of this series
block/monitor/block-hmp-cmds.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
[PATCH] block/monitor/block-hmp-cmds.c: Fix crash when execute hmp_commit
Posted by wangliangzz@126.com 1 year ago
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(-)

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);
-- 
2.27.0
Re: [PATCH] block/monitor/block-hmp-cmds.c: Fix crash when execute hmp_commit
Posted by Emanuele Giuseppe Esposito 1 year ago

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>
Re: [PATCH] block/monitor/block-hmp-cmds.c: Fix crash when execute hmp_commit
Posted by Emanuele Giuseppe Esposito 1 year ago

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
Re: [PATCH] block/monitor/block-hmp-cmds.c: Fix crash when execute hmp_commit
Posted by Wang Liang 1 year ago
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