Bug 15436 - Don't close or flush stdio streams on abort
Summary: Don't close or flush stdio streams on abort
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: stdio (show other bugs)
Version: 2.18
: P2 normal
Target Milestone: 2.27
Assignee: Florian Weimer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-07 10:51 UTC by Andreas Schwab
Modified: 2017-10-05 12:56 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Schwab 2013-05-07 10:51:18 UTC
POSIX doesn't require abort to close or flush any stdio streams, and doing so will eventually run into locking problems, since fclose and fflush are not async-signal-safe, while abort is required to be so.
Comment 1 Zack Weinberg 2015-10-09 18:04:09 UTC
Since glibc has flushed all open files on abort() for a very long time, we probably ought to try to preserve a best-effort flush.

It appears that "stage 1" of abort() calls _IO_flush_all_lockp(0), which attempts to flush all open files *without* doing any locking.  (This strikes me as unsafe in a different manner, but at least it probably can't deadlock.)  However, "stage 4" calls __fcloseall(), which does do locking.  Maybe that should be swapped out for another call to _IO_flush_all_lockp(0)?  Actually *closing* the files seems unnecessary since the only things after stage 4 are an escalating chain of attempts to terminate the process.
Comment 2 Florian Weimer 2017-07-12 00:05:55 UTC
I think this is important security hardening, hence flagging as security+.

At the very least, we should only do this for actual file-based streams (with known vtables), not those resulting from open_memstream or fopencookie.
Comment 3 Florian Weimer 2017-08-17 14:03:45 UTC
I posted a patch which restricts flushing to file-based streams:

https://sourceware.org/ml/libc-alpha/2017-08/msg00761.html
Comment 4 Sourceware Commits 2017-10-05 12:56:34 UTC
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  91e7cf982d0104f0e71770f5ae8e3faf352dea9f (commit)
      from  0c25125780083cbba22ed627756548efe282d1a0 (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=91e7cf982d0104f0e71770f5ae8e3faf352dea9f

commit 91e7cf982d0104f0e71770f5ae8e3faf352dea9f
Author: Florian Weimer <fweimer@redhat.com>
Date:   Thu Oct 5 14:48:16 2017 +0200

    abort: Do not flush stdio streams [BZ #15436]

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

Summary of changes:
 ChangeLog      |    7 +++++++
 NEWS           |    5 +++++
 stdlib/abort.c |   29 +++++------------------------
 3 files changed, 17 insertions(+), 24 deletions(-)
Comment 5 Florian Weimer 2017-10-05 12:56:57 UTC
Fixed in 2.27.