[Bug libc/12847] New: dprintf/vdprintf can cause fork to fail (child process crash)

[Bug libc/12847] New: dprintf/vdprintf can cause fork to fail (child process crash)

[nvbolhuis at aimvalley dot nl]

http://sourceware.org/bugzilla/show_bug.cgi?id=12847

           Summary: dprintf/vdprintf can cause fork to fail (child process
                    crash)
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
        AssignedTo: drepper.fsp@gmail.com
        ReportedBy: nvbolhuis@aimvalley.nl


Created attachment 5769
  --> http://sourceware.org/bugzilla/attachment.cgi?id=5769
demo program that shows fork/system(3) can fail

If a multi-threaded application uses fork and dprintf
(by different threads at about the same time) the fork
can fail because the child process crashes in fresetlockfiles.

This problem can be easily reproduced on our powerpc 32bit
system with the attached program (fork_fail_dprintf.c).
One thread uses system(3) (which is implemented with fork
and exec) to write strings to a file. This often fails.
If write(2) is used instead of dprintf, the problem is gone.

Our system uses glibc-2.7, linux-2.6.28 and has a single core
CPU (Freescale MPC8313).
I tried this with glibc-latest and the problem is still there.
Btw. somehow with glibc-latest the problem also occurs when using
write(2).
The problem does not occur on my host PC (FC8=glibc-2.7
on Intel Core i7).

I believe I found the root-cause of this problem and it looks
to me it's still there in glibc-latest.

dprint adds to the global _IO_list_all a temporary
struct _IO_FILE_plus (tmpfil) for which member _lock is NULL.
If another thread kicks in and calls fork (before tmpfil has
been removed from _IO_list_all!) the child process will
crash in fresetlockfiles. This is because here it will
re-initialize the file locks by writing to the _lock member
(which is NULL!)
see:
http://sourceware.org/ml/libc-help/2011-05/msg00014.html
for the full story.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12847] dprintf/vdprintf can cause fork to fail (child process crash)

[drepper.fsp at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12847

Ulrich Drepper <drepper.fsp at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #5769|application/octet-stream    |text/plain
          mime type|                            |

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12847] dprintf/vdprintf can cause fork to fail (child process crash)

[drepper.fsp at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12847

--- Comment #1 from Ulrich Drepper <drepper.fsp at gmail dot com> 2011-09-09 20:16:14 UTC ---
I checked in a patch.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12847] dprintf/vdprintf can cause fork to fail (child process crash)

[drepper.fsp at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12847

Ulrich Drepper <drepper.fsp at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #2 from Ulrich Drepper <drepper.fsp at gmail dot com> 2011-09-09 20:16:40 UTC ---
Stupid BZ...  Closing again

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12847] dprintf/vdprintf can cause fork to fail (child process crash)

[freker at zes dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12847

Frank Reker <freker at zes dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
                 CC|                            |freker at zes dot com
         Resolution|FIXED                       |---

--- Comment #3 from Frank Reker <freker at zes dot com> ---
Sorry,
maybe I'm blind, but where is the patch?
I still do encounter the problem in version 2.17 from 25th dec. 2012.
The fresetlockfile function (nptl/sysdeps/unix/sysv/linux/fork.c) that fails:
----snip-------
static void
fresetlockfiles (void)
{
  _IO_ITER i;

  for (i = _IO_iter_begin(); i != _IO_iter_end(); i = _IO_iter_next(i))
    _IO_lock_init (*((_IO_lock_t *) _IO_iter_file(i)->_lock));
}
----snap------

and the code in
_IO_vdprintf
----snip-----
  struct _IO_FILE_plus tmpfil;
  struct _IO_wide_data wd;
  int done;

#ifdef _IO_MTSAFE_IO
  tmpfil.file._lock = NULL;
#endif
  _IO_no_init (&tmpfil.file, _IO_USER_LOCK, 0, &wd, &_IO_wfile_jumps);
  _IO_JUMPS (&tmpfil) = &_IO_file_jumps;
  _IO_file_init (&tmpfil);
----snap-----

The _lock here is explicitely set to NULL and _IO_file_init
links the struct to the chain: _IO_list_all

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/12847] dprintf/vdprintf can cause fork to fail (child process crash)

[guillaume at morinfr dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=12847

Guillaume Morin <guillaume at morinfr dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |guillaume at morinfr dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/12847] dprintf/vdprintf can cause fork to fail (child process crash)

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=12847

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fweimer at redhat dot com
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/12847] dprintf/vdprintf can cause fork to fail (child process crash)

[arulnambir at hotmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=12847

Raju <arulnambir at hotmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |arulnambir at hotmail dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/12847] dprintf/vdprintf can cause fork to fail (child process crash)

[f.deldegan at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=12847

Francesco 'pr0gg3d' Del Degan <f.deldegan at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |f.deldegan at gmail dot com

--- Comment #5 from Francesco 'pr0gg3d' Del Degan <f.deldegan at gmail dot com> ---
Hi, I maybe can add another testcase, using syslog.

In one of our program, it happens that a call to syslog() is made and no
/dev/log listener (syslog daemon) is listening.

If i got it correctly, a fallback method in libc/misc/syslog.c is to open
/dev/console:

   298            if (LogStat & LOG_CONS &&
   299                (fd = __open(_PATH_CONSOLE, O_WRONLY|O_NOCTTY, 0)) >= 0)
   300              {
   301                dprintf (fd, "%s\r\n", buf + msgoff);
   302                (void)__close(fd);
   303              }
   304              }

that leads to add a _IO_file into _IO_list_all with a NULL lock, because of
dprintf.

If in meantime, before fd is closed, a fork occurs and the child crashes with a
SEGV at:

static void
fresetlockfiles (void)
{
  _IO_ITER i;

  for (i = _IO_iter_begin(); i != _IO_iter_end(); i = _IO_iter_next(i))
    _IO_lock_init (*((_IO_lock_t *) _IO_iter_file(i)->_lock));
}


because _lock is NULL.

I not have a fix yet to propose, because i just started to dig into libc code,
but i'm wondering if it's correct to call a dprintf there since is not
fork-safe.

Notice that syslog is the *only* libc-internal caller of dprintf.

Thank you,
  Francesco

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/12847] dprintf/vdprintf can cause fork to fail (child process crash)

[f.deldegan at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=12847

Francesco 'pr0gg3d' Del Degan <f.deldegan at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://sourceware.org/bugz
                   |                            |illa/show_bug.cgi?id=16060

--- Comment #6 from Francesco 'pr0gg3d' Del Degan <f.deldegan at gmail dot com> ---
I'm just trying to dig further in the code:

https://sourceware.org/git/?p=glibc.git;a=commit;h=c020d48c6e91b351cefebbc7a82a9c1ec2d9d83b

changed the locks for a bunch of functions.I guess that the intended behaviour
was to mimic a temporary (lightweight?) stream. The lock is set to NULL,
_IO_USER_LOCK is added to _flags but _IO_file_init still called.

_IO_file_init calls _IO_link_in that adds to _IO_list_all file and tries to
lock the fp, but since it's a _IO_USER_LOCK this is not performed.

So __libc_fork makes an hard assumption that all files added into _IO_list_all
have a valid lock. From here, this bug is exposed.

My question for trying to elaborate a patch is:
 It was intended that so called temporary streams gets added to this list?
Moreover, if a fork occurs in the meantime, child leaks a temporary fd that it
will never release.

Could be a fix to add a check for _IO_USER_LOCK in
_IO_new_file_init/_IO_new_file_finish and don't add (and remove) it to the list
at all in this case? Or maybe doing this check at _IO_(un)link_in level could
be more appropriate?

The bad thing around here is that it's fork() that crashes at child because
someone in parent called a dprintf (or syslog()) and it was interrupted before
closing it, thus rendering fork() not safe.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/12847] dprintf/vdprintf can cause fork to fail (child process crash)

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=12847

--- Comment #7 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  217a74a85cdd60df236c296ad88142b78d35eccf (commit)
      from  aa76a5c7010e98c737d79f37aa6ae668f60f7a00 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=217a74a85cdd60df236c296ad88142b78d35eccf

commit 217a74a85cdd60df236c296ad88142b78d35eccf
Author: Andreas Schwab <schwab@suse.de>
Date:   Mon Dec 8 15:13:38 2014 +0100

    Don't touch user-controlled stdio locks in forked child (bug 12847)

    The stdio locks for streams with the _IO_USER_LOCK flag should not be
    touched by internal code.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog           |    6 ++++++
 sysdeps/nptl/fork.c |    3 ++-
 2 files changed, 8 insertions(+), 1 deletions(-)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/12847] dprintf/vdprintf can cause fork to fail (child process crash)

[schwab@linux-m68k.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=12847

Andreas Schwab <schwab@linux-m68k.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Andreas Schwab <schwab@linux-m68k.org> ---
Fixed in 2.21.

-- 
You are receiving this mail because:
You are on the CC list for the bug.