Re: [PATCH] libio: Fix race in _IO_new_file_init_internal initialization order [BZ #33785]

[Florian Weimer <fweimer@redhat.com>]

* Shamil Abdulaev:

> _IO_new_file_init_internal linked the new stream into _IO_list_all
> before setting fp->_fileno to -1.  A concurrent thread that walks
> _IO_list_all (for example via fflush (NULL)) could observe the stream
> with an uninitialized _fileno before initialization completed.
>
> Set _fileno = -1 before _IO_link_in so the stream is fully
> initialized when it becomes visible in the global list.
>
> This is the residual concurrency defect noted at the end of commit
> b657f72fa3 ("libio: Fix deadlock between freopen, fflush (NULL) and
> fclose (bug 24963)").
>
> Add libio/tst-file-init-race exercising concurrent fopen/fclose and
> fflush (NULL) to detect regressions.
>
> Signed-off-by: Shamil Abdulaev <ashamil435@gmail.com>
> ---
>  libio/Makefile             |  3 ++
>  libio/fileops.c            |  2 +-
>  libio/tst-file-init-race.c | 66 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 70 insertions(+), 1 deletion(-)
>  create mode 100644 libio/tst-file-init-race.c
>
> diff --git a/libio/Makefile b/libio/Makefile
> index 93656466df..7e448295e3 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -107,6 +107,7 @@ tests = \
>    tst-fgetc-after-eof \
>    tst-fgetwc \
>    tst-fgetws \
> +  tst-file-init-race \
>    tst-fopenloc2 \
>    tst-fputws \
>    tst-freopen \
> @@ -160,6 +161,8 @@ tests-static += \
>  
>  $(objpfx)tst-popen-fork: $(shared-thread-library)
>  
> +$(objpfx)tst-file-init-race: $(shared-thread-library)
> +
>  tests-internal = tst-vtables tst-vtables-interposed
>  
>  ifeq (yes,$(build-shared))
> diff --git a/libio/fileops.c b/libio/fileops.c
> index 8067c0a9cf..9348d7c3a1 100644
> --- a/libio/fileops.c
> +++ b/libio/fileops.c
> @@ -111,8 +111,8 @@ _IO_new_file_init_internal (struct _IO_FILE_plus *fp)
>    fp->file._offset = _IO_pos_BAD;
>    fp->file._flags |= CLOSED_FILEBUF_FLAGS;
>  
> -  _IO_link_in (fp);
>    fp->file._fileno = -1;
> +  _IO_link_in (fp);
>  }

Thanks for investigating this and fixing it.

Would you please apply the parallel change to libio/oldfileops.c?  It
looks like it has the same bug.

Florian