From f9cfecf3d4d848e247dc8bdc486ed227bff7315f Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Thu, 4 Feb 2021 16:58:13 -0800 Subject: Fix freopen() where the path is null. This has been in the standard since C99, but we've never supported it before. It's apparently used by SPIRV-Tools. I tried implementing this the other way (with fcntl(2)) first, but eventually realized that that's more complicated and gives worse results. This implementation assumes that /proc is mounted, but so much of libc relies on that at this point that I don't think there's any realistic case where the fcntl(2) implementation would be preferable, and there are many where it's not. The fact that no-one's mentioned this until now suggests that it's not a heavily used feature anyway. I've also replaced AssertCloseOnExec() with a CloseOnExec() boolean-valued function instead, because it's really annoying getting assertion failures that don't point you at the test line in question, and instead point to some common helper code. Test: treehugger Change-Id: Ia2e53bf2664a4f782581042054ecd492830e2aed --- libc/stdio/stdio.cpp | 78 +++++++++++++++++++++++++++------------------------- 1 file changed, 41 insertions(+), 37 deletions(-) (limited to 'libc/stdio/stdio.cpp') diff --git a/libc/stdio/stdio.cpp b/libc/stdio/stdio.cpp index c7b1ba459..2b0e2b21f 100644 --- a/libc/stdio/stdio.cpp +++ b/libc/stdio/stdio.cpp @@ -50,11 +50,12 @@ #include -#include "local.h" #include "glue.h" +#include "local.h" +#include "private/ErrnoRestorer.h" +#include "private/FdPath.h" #include "private/__bionic_get_shell_path.h" #include "private/bionic_fortify.h" -#include "private/ErrnoRestorer.h" #include "private/thread_private.h" #include "private/bsd_sys_param.h" // For ALIGN/ALIGNBYTES. @@ -225,25 +226,26 @@ extern "C" __LIBC_HIDDEN__ void __libc_stdio_cleanup(void) { _fwalk(__sflush); } -static FILE* __fopen(int fd, int flags) { +static FILE* __finit(FILE* fp, int fd, int flags) { + if (fp == nullptr) return nullptr; + + fp->_file = fd; + android_fdsan_exchange_owner_tag(fd, 0, __get_file_tag(fp)); + fp->_flags = flags; + fp->_cookie = fp; + fp->_read = __sread; + fp->_write = __swrite; + fp->_close = __sclose; + _EXT(fp)->_seek64 = __sseek64; + #if !defined(__LP64__) if (fd > SHRT_MAX) { errno = EMFILE; + fclose(fp); return nullptr; } #endif - FILE* fp = __sfp(); - if (fp != nullptr) { - fp->_file = fd; - android_fdsan_exchange_owner_tag(fd, 0, __get_file_tag(fp)); - fp->_flags = flags; - fp->_cookie = fp; - fp->_read = __sread; - fp->_write = __swrite; - fp->_close = __sclose; - _EXT(fp)->_seek64 = __sseek64; - } return fp; } @@ -257,14 +259,15 @@ FILE* fopen(const char* file, const char* mode) { return nullptr; } - FILE* fp = __fopen(fd, flags); + FILE* fp = __finit(__sfp(), fd, flags); if (fp == nullptr) { ErrnoRestorer errno_restorer; close(fd); return nullptr; } - // For append mode, even though we use O_APPEND, we need to seek to the end now. + // For append mode, O_APPEND sets the write position for free, but we need to + // set the read position manually. if ((mode_flags & O_APPEND) != 0) __sseek64(fp, 0, SEEK_END); return fp; } @@ -295,15 +298,26 @@ FILE* fdopen(int fd, const char* mode) { fcntl(fd, F_SETFD, tmp | FD_CLOEXEC); } - return __fopen(fd, flags); + return __finit(__sfp(), fd, flags); } -// Re-direct an existing, open (probably) file to some other file. -// ANSI is written such that the original file gets closed if at -// all possible, no matter what. -// TODO: rewrite this mess completely. FILE* freopen(const char* file, const char* mode, FILE* fp) { CHECK_FP(fp); + + // POSIX says: "If pathname is a null pointer, the freopen() function shall + // attempt to change the mode of the stream to that specified by mode, as if + // the name of the file currently associated with the stream had been used. In + // this case, the file descriptor associated with the stream need not be + // closed if the call to freopen() succeeds. It is implementation-defined + // which changes of mode are permitted (if any), and under what + // circumstances." + // + // Linux is quite restrictive about what changes you can make with F_SETFL, + // and in particular won't let you touch the access bits. It's easiest and + // most effective to just rely on /proc/self/fd/... + FdPath fd_path(fp->_file); + if (file == nullptr) file = fd_path.c_str(); + int mode_flags; int flags = __sflags(mode, &mode_flags); if (flags == 0) { @@ -313,6 +327,8 @@ FILE* freopen(const char* file, const char* mode, FILE* fp) { ScopedFileLock sfl(fp); + // TODO: rewrite this mess completely. + // There are actually programs that depend on being able to "freopen" // descriptors that weren't originally open. Keep this from breaking. // Remember whether the stream was open to begin with, and which file @@ -382,24 +398,12 @@ FILE* freopen(const char* file, const char* mode, FILE* fp) { } } - // _file is only a short. - if (fd > SHRT_MAX) { - fp->_flags = 0; // Release. - errno = EMFILE; - return nullptr; - } + fp = __finit(fp, fd, flags); - fp->_flags = flags; - fp->_file = fd; - android_fdsan_exchange_owner_tag(fd, 0, __get_file_tag(fp)); - fp->_cookie = fp; - fp->_read = __sread; - fp->_write = __swrite; - fp->_close = __sclose; - _EXT(fp)->_seek64 = __sseek64; + // For append mode, O_APPEND sets the write position for free, but we need to + // set the read position manually. + if (fp && (mode_flags & O_APPEND) != 0) __sseek64(fp, 0, SEEK_END); - // For append mode, even though we use O_APPEND, we need to seek to the end now. - if ((mode_flags & O_APPEND) != 0) __sseek64(fp, 0, SEEK_END); return fp; } __strong_alias(freopen64, freopen); -- cgit v1.2.3