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

["f.deldegan at gmail dot com" <sourceware-bugzilla at sourceware dot org>]

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.