Merge "Make FILE*s less usable after fclose(3)."
diff --git a/libc/Android.mk b/libc/Android.mk
index 375b342..45f419a 100644
--- a/libc/Android.mk
+++ b/libc/Android.mk
@@ -444,7 +444,6 @@
upstream-openbsd/lib/libc/stdio/asprintf.c \
upstream-openbsd/lib/libc/stdio/clrerr.c \
upstream-openbsd/lib/libc/stdio/dprintf.c \
- upstream-openbsd/lib/libc/stdio/fclose.c \
upstream-openbsd/lib/libc/stdio/fdopen.c \
upstream-openbsd/lib/libc/stdio/feof.c \
upstream-openbsd/lib/libc/stdio/ferror.c \
@@ -455,7 +454,6 @@
upstream-openbsd/lib/libc/stdio/fgets.c \
upstream-openbsd/lib/libc/stdio/fgetwc.c \
upstream-openbsd/lib/libc/stdio/fgetws.c \
- upstream-openbsd/lib/libc/stdio/fileno.c \
upstream-openbsd/lib/libc/stdio/flags.c \
upstream-openbsd/lib/libc/stdio/fmemopen.c \
upstream-openbsd/lib/libc/stdio/fopen.c \
@@ -1623,4 +1621,3 @@
LOCAL_SANITIZE := never
LOCAL_NATIVE_COVERAGE := $(bionic_coverage)
include $(BUILD_STATIC_LIBRARY)
-
diff --git a/libc/stdio/findfp.c b/libc/stdio/findfp.c
index 6e20562..2a6be15 100644
--- a/libc/stdio/findfp.c
+++ b/libc/stdio/findfp.c
@@ -157,3 +157,37 @@
/* (void) _fwalk(fclose); */
(void) _fwalk(__sflush); /* `cheating' */
}
+
+int fclose(FILE* fp) {
+ if (fp->_flags == 0) {
+ // Already freed!
+ errno = EBADF;
+ return EOF;
+ }
+
+ FLOCKFILE(fp);
+ WCIO_FREE(fp);
+ int r = fp->_flags & __SWR ? __sflush(fp) : 0;
+ if (fp->_close != NULL && (*fp->_close)(fp->_cookie) < 0) {
+ r = EOF;
+ }
+ if (fp->_flags & __SMBF) free(fp->_bf._base);
+ if (HASUB(fp)) FREEUB(fp);
+ if (HASLB(fp)) FREELB(fp);
+
+ // Poison this FILE so accesses after fclose will be obvious.
+ fp->_file = -1;
+ fp->_r = fp->_w = 0;
+
+ // Release this FILE for reuse.
+ fp->_flags = 0;
+ FUNLOCKFILE(fp);
+ return (r);
+}
+
+int fileno(FILE* fp) {
+ FLOCKFILE(fp);
+ int result = fileno_unlocked(fp);
+ FUNLOCKFILE(fp);
+ return result;
+}
diff --git a/libc/stdio/local.h b/libc/stdio/local.h
index 6dcd3ae..a15bddf 100644
--- a/libc/stdio/local.h
+++ b/libc/stdio/local.h
@@ -215,7 +215,6 @@
#define __sfeof(p) (((p)->_flags & __SEOF) != 0)
#define __sferror(p) (((p)->_flags & __SERR) != 0)
#define __sclearerr(p) ((void)((p)->_flags &= ~(__SERR|__SEOF)))
-#define __sfileno(p) ((p)->_file)
#if !defined(__cplusplus)
#define __sgetc(p) (--(p)->_r < 0 ? __srget(p) : (int)(*(p)->_p++))
static __inline int __sputc(int _c, FILE* _p) {
diff --git a/libc/stdio/stdio_ext.cpp b/libc/stdio/stdio_ext.cpp
index f273d45..88e5951 100644
--- a/libc/stdio/stdio_ext.cpp
+++ b/libc/stdio/stdio_ext.cpp
@@ -27,6 +27,8 @@
*/
#include <stdio_ext.h>
+
+#include <errno.h>
#include <stdlib.h>
#include "local.h"
@@ -101,5 +103,10 @@
}
int fileno_unlocked(FILE* fp) {
- return __sfileno(fp);
+ int fd = fp->_file;
+ if (fd == -1) {
+ errno = EBADF;
+ return -1;
+ }
+ return fd;
}
diff --git a/libc/upstream-openbsd/lib/libc/stdio/fclose.c b/libc/upstream-openbsd/lib/libc/stdio/fclose.c
deleted file mode 100644
index c72af54..0000000
--- a/libc/upstream-openbsd/lib/libc/stdio/fclose.c
+++ /dev/null
@@ -1,63 +0,0 @@
-/* $OpenBSD: fclose.c,v 1.9 2009/11/09 00:18:27 kurt Exp $ */
-/*-
- * Copyright (c) 1990, 1993
- * The Regents of the University of California. All rights reserved.
- *
- * This code is derived from software contributed to Berkeley by
- * Chris Torek.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- * may be used to endorse or promote products derived from this software
- * without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#include <errno.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include "local.h"
-
-int
-fclose(FILE *fp)
-{
- int r;
-
- if (fp->_flags == 0) { /* not open! */
- errno = EBADF;
- return (EOF);
- }
- FLOCKFILE(fp);
- WCIO_FREE(fp);
- r = fp->_flags & __SWR ? __sflush(fp) : 0;
- if (fp->_close != NULL && (*fp->_close)(fp->_cookie) < 0)
- r = EOF;
- if (fp->_flags & __SMBF)
- free((char *)fp->_bf._base);
- if (HASUB(fp))
- FREEUB(fp);
- if (HASLB(fp))
- FREELB(fp);
- fp->_r = fp->_w = 0; /* Mess up if reaccessed. */
- fp->_flags = 0; /* Release this FILE for reuse. */
- FUNLOCKFILE(fp);
- return (r);
-}
diff --git a/libc/upstream-openbsd/lib/libc/stdio/fileno.c b/libc/upstream-openbsd/lib/libc/stdio/fileno.c
deleted file mode 100644
index 58628da..0000000
--- a/libc/upstream-openbsd/lib/libc/stdio/fileno.c
+++ /dev/null
@@ -1,51 +0,0 @@
-/* $OpenBSD: fileno.c,v 1.8 2009/11/09 00:18:27 kurt Exp $ */
-/*-
- * Copyright (c) 1990, 1993
- * The Regents of the University of California. All rights reserved.
- *
- * This code is derived from software contributed to Berkeley by
- * Chris Torek.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- * may be used to endorse or promote products derived from this software
- * without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#include <stdio.h>
-#include "local.h"
-
-/*
- * A subroutine version of the macro fileno.
- */
-#undef fileno
-
-int
-fileno(FILE *fp)
-{
- int ret;
-
- FLOCKFILE(fp);
- ret = __sfileno(fp);
- FUNLOCKFILE(fp);
- return (ret);
-}
diff --git a/tests/stdio_test.cpp b/tests/stdio_test.cpp
index 58109a9..d054bf5 100644
--- a/tests/stdio_test.cpp
+++ b/tests/stdio_test.cpp
@@ -1054,3 +1054,17 @@
fclose(fr);
fclose(fw);
}
+
+TEST(STDIO_TEST, fclose_invalidates_fd) {
+ // The typical error we're trying to help people catch involves accessing
+ // memory after it's been freed. But we know that stdin/stdout/stderr are
+ // special and don't get deallocated, so this test uses stdin.
+ ASSERT_EQ(0, fclose(stdin));
+
+ // Even though using a FILE* after close is undefined behavior, I've closed
+ // this bug as "WAI" too many times. We shouldn't hand out stale fds,
+ // especially because they might actually correspond to a real stream.
+ errno = 0;
+ ASSERT_EQ(-1, fileno(stdin));
+ ASSERT_EQ(EBADF, errno);
+}