This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH RFC] libio: Fix deadlock between freopen and fclose [BZ #24963]


On 21/09/2019 04:21, 谢宜生(毅晟) wrote:
> we find a deadlock between fclose and freopen as following:
> CPU0                                          CPU1
> freopen                                       fclose
>   ->_IO_acquire_lock (fp)                         ->_IO_un_link
>   ->_IO_file_close_it                              ->_IO_lock_lock(list_all_lock)
>     ->_IO_un_link
>       ->_IO_lock_lock (list_all_lock)<-wait here
>                                                    ->_IO_flockfile((_IO_FILE *) fp); <-- wait here
> 
> As Carlos pointed that this maybe the bug of in _IO_new_fclose, which
> can be fixed by locking fp first, then with fp acquired lock the whole
> list.

if we want to keep freopen as is then an fp cannot
be locked after list_all_lock is held, if another
thread might freopen(fp).

this affects more than just fclose, e.g.
fflush(NULL) would have the same lock ordering
(first locks list_all_lock, then locks each fp)

this tells me that a different fix is needed,
either way there should be a bug report about
it in bugzilla (and the id referenced in the
changelog of a patch submission assuming you
have the fsf copyright assignment sorted out).

> 
> Signed-off-by: Yisheng Xie <yisheng.xys@alibaba-inc.com>
> ---
>  libio/iofclose.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libio/iofclose.c b/libio/iofclose.c
> index 398b86d597..fe262cf6aa 100644
> --- a/libio/iofclose.c
> +++ b/libio/iofclose.c
> @@ -44,11 +44,11 @@ _IO_new_fclose (FILE *fp)
>      return _IO_old_fclose (fp);
>  #endif
> 
> +  _IO_acquire_lock (fp);
>    /* First unlink the stream.  */
>    if (fp->_flags & _IO_IS_FILEBUF)
>      _IO_un_link ((struct _IO_FILE_plus *) fp);
> 
> -  _IO_acquire_lock (fp);
>    if (fp->_flags & _IO_IS_FILEBUF)
>      status = _IO_file_close_it (fp);
>    else
> --
> 2.23.0
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]