Merge "Make FILE*s less usable after fclose(3)."

libc/Android.mk

@@ -444,7 +444,6 @@ libc_upstream_openbsd_ndk_src_files := \
     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 @@ libc_upstream_openbsd_ndk_src_files := \
     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_SYSTEM_SHARED_LIBRARIES := libc
 LOCAL_SANITIZE := never
 LOCAL_NATIVE_COVERAGE := $(bionic_coverage)
 include $(BUILD_STATIC_LIBRARY)
-

libc/stdio/findfp.c

@@ -157,3 +157,37 @@ __LIBC_HIDDEN__ void __libc_stdio_cleanup(void) {
 	/* (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;
+}

libc/stdio/local.h

@@ -215,7 +215,6 @@ int	__vfwscanf(FILE * __restrict, const wchar_t * __restrict, __va_list);
 #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) {

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 ferror_unlocked(FILE* fp) {
 }
 
 int fileno_unlocked(FILE* fp) {
-  return __sfileno(fp);
+  int fd = fp->_file;
+  if (fd == -1) {
+    errno = EBADF;
+    return -1;
+  }
+  return fd;
 }

libc/upstream-openbsd/lib/libc/stdio/fclose.c

@@ -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);
-}

libc/upstream-openbsd/lib/libc/stdio/fileno.c

@@ -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);
-}

tests/stdio_test.cpp

@@ -1054,3 +1054,17 @@ TEST(STDIO_TEST, fread_EOF_184847) {
   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);
+}