Re: [Qemu-devel] [PATCHv4 01/03] qemu-iothread: IOThread supports theGMainContext event loop

Re: [Qemu-devel] [PATCHv4 01/03] qemu-iothread: IOThread supports theGMainContext event loop

[wang.yong155]

>Hi Wang Yong,>>To make the discussion easier, please try to fix your email client to:>>1) set In-Reply-To: header when replying>2) use plain text instead of html>3) use monospace fonts to view and compose a reply>4) avoid attaching the original email in the end, just reply inline>5) maybe, use "Re:" in the subject for reply, avoid "答复:">6) include not only email addresses in From:To:/Cc: headers, but also>   the names of recipients, in the form of>>       Some Body <some.body@example.com>, Another One <another.one@example.com>,>       ...>Or maybe just switch to a functional email client.

Hi Fam,I am very sorry that our company's mail client does not support the part.






>> I think it's ok, I don't know whether I understand it correctly or not?>>Your sequence is ok. But remember this is multi-threaded and the execution order>between two threads are non-deterministic. The sequence I pointed out is also>"possible" and will cause use-after-free due to TOCTOU race condition [1].>>[1]: https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use

Thanks,When iothread_stop is called two times concurrently,this BUG may be triggered.

I will submit the patch v5.




WangYong









原始邮件



发件人： <famz@redhat.com>
收件人：王勇10170530
抄送人： <pbonzini@redhat.com> <stefanha@redhat.com> <jasowang@redhat.com> <zhangchen.fnst@cn.fujitsu.com> <zhang.zhanghailiang@huawei.com>王广10165992 <lizhijian@cn.fujitsu.com> <qemu-devel@nongnu.org>
日 期 ：2017年08月23日 16:43
主 题 ：Re: 答复: Re: [PATCHv4 01/03] qemu-iothread: IOThread supports theGMainContext event loop





Hi Wang Yong,

To make the discussion easier, please try to fix your email client to:

1) set In-Reply-To: header when replying
2) use plain text instead of html
3) use monospace fonts to view and compose a reply
4) avoid attaching the original email in the end, just reply inline
5) maybe, use "Re:" in the subject for reply, avoid "答复:"
6) include not only email addresses in From:To:/Cc: headers, but also
   the names of recipients, in the form of

       Some Body <some.body@example.com>, Another One <another.one@example.com>,
       ...

Or maybe just switch to a functional email client.

On Wed, 08/23 15:58, wang.yong155@zte.com.cn wrote:
> >> diff --git a/iothread.c b/iothread.c>> index beeb870..fb1c55b 100644>> --- a/iothread.c>> +++ b/iothread.c>> @@ -57,6 +57,20 @@ static void *iothread_run(void *opaque)>>  >>      while (!atomic_read(&iothread->stopping)) {>>          aio_poll(iothread->ctx, true)>> +>> +        if (atomic_read(&iothread->worker_context)) {>> +            g_main_context_push_thread_default(iothread->worker_context)>> +            iothread->main_loop =>> +                g_main_loop_new(iothread->worker_context, TRUE)>> +            g_main_loop_run(iothread->main_loop)>> +>> +            g_main_loop_unref(iothread->main_loop)>> +            iothread->main_loop = NULL>
> 
> >You should clear iothread->main_loop first before calling g_main_loop_unref(),>to avoid TOCTOU race with iothread_stop():>
> 
> >  iothread_run (in IOThread)          iothread_stop (in main thread)> ========================================================================>                                    if (atomic_read(&iothread->main_loop)) {>  /* frees iothread->main_loop */>  g_main_loop_unref(...)>                                         /* Accesses freed memory */>                                          g_main_loop_quit(iothread->main_loop)>                                      }>  iothread->main_loop = NULL
> 
> When the g_main_loop_quit function is called, the g_main_loop_run function can exit?
> 
> 
> 
> 
> iothread_run (in IOThread)                                         iothread_stop (in main thread)
> 
> ========================================================================
> 
>                                                                                    /*step1: set  loop->is_running = FALSE*/
> 
>                                                                                   g_main_loop_quit(iothread->main_loop)
> 
> /*step2: main loop exit */                                                                                                                                                                    
> 
> g_main_loop_run()
> 
> /*step3:frees iothread->main_loop memory*/
> 
> g_main_loop_unref(...)
> 
> iothread->main_loop = NULL
> 
> 
> 
> 
> I think it's ok, I don't know whether I understand it correctly or not?

Your sequence is ok. But remember this is multi-threaded and the execution order
between two threads are non-deterministic. The sequence I pointed out is also
"possible" and will cause use-after-free due to TOCTOU race condition [1].

[1]: https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use

Fam