This fixes a buffer overflow in wide character string output, reproducing
when output fails, such as if the output fd is closed or is redirected
to a full device.
Wide character output data attempts to maintain the invariant that
`_IO_buf_base <= _IO_write_base <= _IO_write_end <= _IO_buf_end` (that is,
that the write region is a sub-region of `_IO_buf`). Prior to this commit,
this invariant is violated by the `_IO_wfile_overflow` function as so:
1. `_IO_wsetg` is called, assigning `_IO_write_base` to `_IO_buf_base`
2. `_IO_doallocbuf` is called, which jumps to `_IO_wfile_doallocate` via
the _IO_wfile_jumps vtable. This function then assigns the wide data
`_IO_buf_base` and `_IO_buf_end` to a malloc'd buffer.
Thus the invariant is violated. The fix is simply to reverse the order:
malloc the `_IO_buf` first and then assign `_IO_write_base` to it.
We also take this opportunity to defensively guard the initialization of
the number of unwritten characters via pointer arithmetic. We now check
that the buffer end is not before the buffer beginning; this matches a
similar defensive check in the narrow analogue `fileops.c`.
Add a test which fails without the fix.
Signed-off-by: Peter Ammon <corydoras@ridiculousfish.com>
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
tst-popen-fork failed to build for Hurd due to not being linked with
libpthread. This commit fixes that.
Tested with build-many-glibcs.py for i686-gnu.
Reviewed-by: Florian Weimer <fweimer@redhat.com>
popen modifies its file handler book-keeping under a lock that wasn't
being taken during fork. This meant that a concurrent popen and fork
could end up copying the lock in a "locked" state into the fork child,
where subsequently calling popen would lead to a deadlock due to the
already (spuriously) held lock.
This commit fixes the deadlock by appropriately taking the lock before
fork, and releasing/resetting it in the parent/child after the fork.
A new test for concurrent popen and fork is also added. It consistently
hangs (and therefore fails via timeout) without the fix applied.
Reviewed-by: Florian Weimer <fweimer@redhat.com>
Since _IO_vtable_offset is used to detect the old binaries, set it
in _IO_old_file_init_internal before calling _IO_link_in which checks
_IO_vtable_offset. Add a glibc 2.0 test with copy relocation on
_IO_stderr_@GLIBC_2.0 to verify that fopen won't cause memory corruption.
This fixes BZ #32148.
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Reviewed-by: Noah Goldstein <goldstein.w.n@gmail.com>
Add new file libio/tst-fclose-unopened2.c that tests whether fclose on an
unopened file returns EOF.
This test differs from tst-fclose-unopened.c by ensuring the file's buffer
is allocated prior to double-fclose. A comment in tst-fclose-unopened.c
now clarifies that it is testing a file with an unallocated buffer.
Calling fclose on unopened files normally causes a use-after-free bug,
however the standard streams are an exception since they are not
deallocated by fclose.
Tested for x86_64.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
As reported in bug 23675 and shown up in the recently added tests of
different cases of freopen (relevant part of the test currently
conditioned under #if 0 to avoid a failure resulting from this bug),
freopen wrongly forces the stream to unoriented even when a mode with
,ccs= is specified, though such a mode is supposed to result in a
wide-oriented stream. Move the clearing of _mode to before the actual
reopening occurs, so that the main fopen implementation can leave a
wide-oriented stream in the ,ccs= case.
Tested for x86_64.
Add new file libio/tst-fclosed-unopened.c that tests whether fclose on
an unopened file returns EOF.
Calling fclose on unopened files normally causes a use-after-free bug,
however the standard streams are an exception since they are not
deallocated by fclose.
fclose returning EOF for unopened files is not part of the external
contract but there are dependancies on this behaviour. For example,
gnulib's close_stdout in lib/closeout.c.
Tested for x86_64.
Signed-off-by: Aaron Merey <amerey@redhat.com>
As reported in bug 32140, freopen leaks the FILE object when it
returns NULL: there is no valid use of the FILE * pointer (including
passing to freopen again or to fclose) after such an error return, so
the underlying object should be freed. Add code to free it.
Note 1: while I think it's clear from the relevant standards that the
object should be freed and the FILE * can't be used after the call in
this case (the stream is closed, which ends the lifetime of the FILE),
it's entirely possible that some existing code does in fact try to use
the existing FILE * in some way and could be broken by this change.
(Though the most common case for freopen may be stdin / stdout /
stderr, which _IO_deallocate_file explicitly checks for and does not
deallocate.)
Note 2: the deallocation is only done in the _IO_IS_FILEBUF case.
Other kinds of streams bypass all the freopen logic handling closing
the file, meaning a call to _IO_deallocate_file would neither be safe
(the FILE might still be linked into the list of all open FILEs) nor
sufficient (other internal memory allocations associated with the file
would not have been freed). I think the validity of freopen for any
other kind of stream will need clarifying with the Austin Group, but
if it is valid in any such case (where "valid" means "not undefined
behavior so required to close the stream" rather than "required to
successfully associate the stream with the new file in cases where
fopen would work"), more significant changes would be needed to ensure
the stream gets fully closed.
Tested for x86_64.
As reported in bug 32134, freopen does not clear the flags set in
fp->_flags2 by the "e", "m" or "c" mode characters. Clear these so
that they can be set or not as appropriate from the mode string passed
to freopen. The relevant test for "e" in tst-freopen2-main.c is
enabled accordingly; "c" is expected to be covered in a separately
written test (and while tst-freopen2-main.c does include transitions
to and from "m", that's not really a semantic flag intended to result
in behaving in an observably different way).
Tested for x86_64.
_wide_data and _mode are not available in legacy code, so do not attempt
to free the wide backup buffer in legacy code.
Resolves: BZ #32137 and BZ #27821
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: Florian Weimer <fweimer@redhat.com>
If a file descriptor is left unclosed and is cleaned up by _IO_cleanup
on exit, its backup buffer remains unfreed, registering as a leak in
valgrind. This is not strictly an issue since (1) the program should
ideally be closing the stream once it's not in use and (2) the program
is about to exit anyway, so keeping the backup buffer around a wee bit
longer isn't a real problem. Free it anyway to keep valgrind happy
when the streams in question are the standard ones, i.e. stdout, stdin
or stderr.
Also, the _IO_have_backup macro checks for _IO_save_base,
which is a roundabout way to check for a backup buffer instead of
directly looking for _IO_backup_base. The roundabout check breaks when
the main get area has not been used and user pushes a char into the
backup buffer with ungetc. Fix this to use the _IO_backup_base
directly.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
When ungetc is called on an unused stream, the backup buffer is
allocated without the main get area being present. This results in
every subsequent ungetc (as the stream remains in the backup area)
checking uninitialized memory in the backup buffer when trying to put a
character back into the stream.
Avoid comparing the input character with buffer contents when in backup
to avoid this uninitialized read. The uninitialized read is harmless in
this context since the location is promptly overwritten with the input
character, thus fulfilling ungetc functionality.
Also adjust wording in the manual to drop the paragraph that says glibc
cannot do multiple ungetc back to back since with this change, ungetc
can actually do this.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
The conditionals for several mtrace-based tests in catgets, elf, libio,
malloc, misc, nptl, posix, and stdio-common were incorrect leading to
test failures when bootstrapping glibc without perl.
The correct conditional for mtrace-based tests requires three checks:
first checking for run-built-tests, then build-shared, and lastly that
PERL is not equal to "no" (missing perl).
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
These comments were written in 2003 (added in 2c008571c3), predating
the addition of getdelim(3)/getline(3) in POSIX.1-2008.
Reviewed-by: Sam James <sam@gentoo.org>
Reviewed-by: Florian Weimer <fweimer@redhat.com>
Deallocate the memory for the FILE structure when seeking to the end fails
in append mode.
Fixes: ea33158c96 ("Fix offset caching for streams and use it for ftell (BZ #16680)")
Since Glibc never provides symbol binary compatibility for relocatable
files, fix BZ #31766 by changing _IO_stderr_/_IO_stdin_/_IO_stdout to
compat symbols.
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Reviewed-by: Sunil K Pandey <skpgkp2@gmail.com>
This patch fixes BZ #27777 "fclose does a linear search, takes ages when
many FILE* are opened". Simply put, the master list of opened (FILE*),
namely _IO_list_all, is a singly-linked list. As a consequence, the
removal of a single element is in O(N), which cripples the performance
of fclose(). The patch switches to a doubly-linked list, yielding O(1)
removal. The one padding field in struct _IO_FILE, __pad5, is renamed
to _prevchain for a doubly-linked list. Since fields in struct _IO_FILE
after the _lock field are internal to glibc and opaque to applications.
We can change them as long as the size of struct _IO_FILE is unchanged,
which is checked as the part of glibc ABI with sizes of _IO_2_1_stdin_,
_IO_2_1_stdout_ and _IO_2_1_stderr_.
NB: When _IO_vtable_offset (fp) == 0, copy relocation will cover the
whole struct _IO_FILE. Otherwise, only fields up to the _lock field
will be copied to applications at run-time. It is used to check if
the _prevchain field can be safely accessed.
After opening 2 million (FILE*), the fclose() of 100 of them takes quite
a few seconds without the patch, and under 2 seconds with it on a loaded
machine.
No test is added since there are no functional changes.
Co-Authored-By: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Alexandre Ferrieux <alexandre.ferrieux@orange.com>
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
This test case calls `fopen':
FILE *fp = fopen (temp_file, "r");
however if that fails it reports `fdopen' being the origin of the error.
Adjust the message to say `fopen' then.
Starting from glibc 2.1, crt1.o contains _IO_stdin_used which is checked
by _IO_check_libio to provide binary compatibility for glibc 2.0. Add
crt1-2.0.o for tests against glibc 2.0. Define tests-2.0 for glibc 2.0
compatibility tests. Add and update glibc 2.0 compatibility tests for
stderr, matherr and pthread_kill.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
The gnulib version contains an important change (9ce573cde), which
fixes some problems with multithreading, entropy loss, and ASLR leak
nfo. It also fixes an issue where getrandom is not being used
on some new files generation (only for __GT_NOCREATE on first try).
The 044bf893ac removed __path_search, which is now moved to another
gnulib shared files (stdio-common/tmpdir.{c,h}). Tthis patch
also fixes direxists to use __stat64_time64 instead of __xstat64,
and move the include of pathmax.h for !_LIBC (since it is not used
by glibc). The license is also changed from GPL 3.0 to 2.1, with
permission from the authors (Bruno Haible and Paul Eggert).
The sync also removed the clock fallback, since clock_gettime
with CLOCK_REALTIME is expected to always succeed.
It syncs with gnulib commit 323834962817af7b115187e8c9a833437f8d20ec.
Checked on x86_64-linux-gnu.
Co-authored-by: Bruno Haible <bruno@clisp.org>
Co-authored-by: Paul Eggert <eggert@cs.ucla.edu>
Reviewed-by: Bruno Haible <bruno@clisp.org>
We would like to avoid statically defining any specific page size on
aarch64-gnu, and instead make sure that everything uses the dynamic
page size, available via vm_page_size and GLRO(dl_pagesize).
There are currently a few places in glibc that require EXEC_PAGESIZE
to be defined. Per Roland's suggestion [0], drop the static
GLRO(dl_pagesize) initializers (for now, only if EXEC_PAGESIZE is not
defined), and don't require EXEC_PAGESIZE definition for libio to
enable mmap usage.
[0]: https://mail.gnu.org/archive/html/bug-hurd/2011-10/msg00035.html
Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
Message-ID: <20240323173301.151066-4-bugaevc@gmail.com>
It improve fortify checks for sprintf, vsprintf, vsnsprintf, fprintf,
dprintf, asprintf, __asprintf, obstack_printf, gets, fgets,
fgets_unlocked, fread, and fread_unlocked. The runtime checks have
similar support coverage as with GCC.
For function with variadic argument (sprintf, snprintf, fprintf, printf,
dprintf, asprintf, __asprintf, obstack_printf) the fortify wrapper calls
the va_arg version since clang does not support __va_arg_pack.
Checked on aarch64, armhf, x86_64, and i686.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Tested-by: Carlos O'Donell <carlos@redhat.com>
WG14 decided to use the name C23 as the informal name of the next
revision of the C standard (notwithstanding the publication date in
2024). Update references to C2X in glibc to use the C23 name.
This is intended to update everything *except* where it involves
renaming files (the changes involving renaming tests are intended to
be done separately). In the case of the _ISOC2X_SOURCE feature test
macro - the only user-visible interface involved - support for that
macro is kept for backwards compatibility, while adding
_ISOC23_SOURCE.
Tested for x86_64.
The multibyte character needs to fit into the remaining buffer space,
not the already-written buffer space. Without the fix, we were never
moving the write pointer from the start of the buffer, always using
the single-character fallback buffer.
Fixes commit 04b76b5aa8 ("Don't error out writing
a multibyte character to an unbuffered stream (bug 17522)").
During the review of a GCC analyzer test case, we found most stdio
functions accepting a FILE * argument expect it to be nonnull and just
segfault when the argument is NULL. Add nonnull attribute for them.
fflush and fflush_unlocked are well defined when __stream is NULL so
they are not touched.
For fputs, fgets, fread, fwrite, fprintf, vfprintf, and their unlocked
version, if __stream is empty but there is nothing to read or write,
they did not segfault. But the standard disallow __stream to be empty
here, so nonnull attribute is also added for them. Note that this may
blow up some old code already subtly broken.
Also add __nonnull for _chk variants and __fortify_function versions for
them.
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
IO_VTABLES_LEN is the size of the struct array in bytes, not the number
of __IO_jump_t's in the array. Drops just under 384kb from .rodata on
LP64 machines.
Fixes: 3020f72618 ("libio: Remove the usage of __libc_IO_vtables")
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Florian Weimer <fweimer@redhat.com>
Tested-by: Florian Weimer <fweimer@redhat.com>
On GNU/Hurd, O_RDWR actually is O_WRONLY|O_RDONLY, so checking through
bitness really is wrong. O_ACCMODE is there for this.
Fixes: 5324d25842 ("fileops: Don't process ,ccs= as individual mode flags (BZ#18906)")
In processing the first 7 individual characters of the mode for fopen
if ,ccs= is used those characters will be processed as well. Stop
processing individual mode flags once a comma is encountered. This has
the effect of requiring ,ccs= to be the last mode flag in the mode
string. Add a testcase to check that the ,ccs= mode flag is not
processed as individual mode flags.
Reviewed-by: DJ Delorie <dj@redhat.com>
The change is meant to avoid unwanted PLT entry for the fgets_unlocked
routine when _FORTIFY_SOURCE is set.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Move declarations from libio/bits/stdio.h to existing
libio/bits/stdio2-decl.h. This will enable future use of
__REDIRECT_FORTIFY in place of some __REDIRECT.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Since the _FORTIFY_SOURCE feature uses some routines of Glibc, they need to
be excluded from the fortification.
On top of that:
- some tests explicitly verify that some level of fortification works
appropriately, we therefore shouldn't modify the level set for them.
- some objects need to be build with optimization disabled, which
prevents _FORTIFY_SOURCE to be used for them.
Assembler files that implement architecture specific versions of the
fortified routines were not excluded from _FORTIFY_SOURCE as there is no
C header included that would impact their behavior.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Now that abort no longer calls fflush there is no reason to avoid locking
the stdio streams anywhere. This fixes a conformance issue and potential
heap corruption during exit.
With fortification enabled, system calls return result needs to be checked,
has it gets the __wur macro enabled.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
With fortification enabled, fread calls return result needs to be checked,
has it gets the __wur macro enabled.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Calling fclose or freopen with a null FILE * is undefined behavior, and
doing so in practice will cause a SIGSEGV. So it seems suitable for
__nonnull.
This will help the compiler to warn for some buggy code, like
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
GCC docs explicitly list perror () as a good candidate for using
__attribute__ ((cold)). So apply __COLD to perror () and similar
functions.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
Message-Id: <20230429131223.2507236-3-bugaevc@gmail.com>
FreeBSD makes these functions available by default, so we should
not treat them as GNU-specific and restrict them to _GNU_SOURCE.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Prevent sh from interpreting a user string as shell options if it
starts with '-' or '+'. Since the version of /bin/sh used for testing
system() is different from the full-fledged system /bin/sh add support
to it for handling "--" after "-c". Add a testcase to ensure the
expected behavior.
Signed-off-by: Joe Simmons-Talbott <josimmon@redhat.com>
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Instead of using a special ELF section along with a linker script
directive to put the IO vtables within the RELRO section, the libio
vtables are all moved to an array marked as data.relro (so linker
will place in the RELRO segment without the need of extra directives).
To avoid static linking namespace issues and including all vtable
referenced objects, all required function pointers are set to weak alias.
Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Instead define the required fields in system dependend files. The only
system dependent definition is FILENAME_MAX, which should match POSIX
PATH_MAX, and it is obtained from either kernel UAPI or mach headers.
Currently set pre-defined value from current kernels.
It avoids a circular dependendy when including stdio.h in
gen-as-const-headers files.
Checked on x86_64-linux-gnu and i686-linux-gnu
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
