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 <address@hidden>, Another One <address@hidden>,>       ...>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









原始邮件



发件人： <address@hidden>
收件人：王勇10170530
抄送人： <address@hidden> <address@hidden> <address@hidden> <address@hidden> 
<address@hidden>王广10165992 <address@hidden> <address@hidden>
日 期 ：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 <address@hidden>, Another One <address@hidden>,
       ...

Or maybe just switch to a functional email client.

On Wed, 08/23 15:58, address@hidden 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