diff --git a/fs/inode/fs_files.c b/fs/inode/fs_files.c index 1f91b5e51e751..8db30136f20f3 100644 --- a/fs/inode/fs_files.c +++ b/fs/inode/fs_files.c @@ -37,6 +37,10 @@ #include #include +#ifdef CONFIG_FDSAN +# include +#endif + #include "inode/inode.h" /**************************************************************************** @@ -686,6 +690,10 @@ int close(int fd) { int ret; +#ifdef CONFIG_FDSAN + android_fdsan_exchange_owner_tag(fd, 0, 0); +#endif + /* close() is a cancellation point */ enter_cancellation_point(); diff --git a/fs/vfs/fs_ioctl.c b/fs/vfs/fs_ioctl.c index 2fcf0e012416b..8788f6b2805bc 100644 --- a/fs/vfs/fs_ioctl.c +++ b/fs/vfs/fs_ioctl.c @@ -43,6 +43,9 @@ int file_vioctl(FAR struct file *filep, int req, va_list ap) { FAR struct inode *inode; +#ifdef CONFIG_FDSAN + FAR uint64_t *tag; +#endif unsigned long arg; int ret = -ENOTTY; @@ -109,6 +112,20 @@ int file_vioctl(FAR struct file *filep, int req, va_list ap) } break; +#ifdef CONFIG_FDSAN + case FIOC_SETTAG: + tag = (FAR uint64_t *)arg; + filep->f_tag = *tag; + ret = OK; + break; + + case FIOC_GETTAG: + tag = (FAR uint64_t *)arg; + *tag = filep->f_tag; + ret = OK; + break; +#endif + #ifndef CONFIG_DISABLE_MOUNTPOINT case BIOC_BLKSSZGET: if (ret == -ENOTTY && inode->u.i_ops != NULL && diff --git a/include/android/fdsan.h b/include/android/fdsan.h new file mode 100644 index 0000000000000..4515914a81d1e --- /dev/null +++ b/include/android/fdsan.h @@ -0,0 +1,190 @@ +/**************************************************************************** + * include/android/fdsan.h + * Copyright (C) 2018 The Android Open Source Project + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * 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. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS 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 + * COPYRIGHT OWNER 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. + ****************************************************************************/ + + #ifndef __INCLUDE_ANDROID_FDSAN_H + #define __INCLUDE_ANDROID_FDSAN_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include +#include + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/**************************************************************************** + * Error checking for close(2). + * + * Mishandling of file descriptor ownership is a common source of errors that + * can be extremely difficult to diagnose. Mistakes like the following can + * result in seemingly 'impossible' failures showing up on other threads that + * happened to try to open a file descriptor + * between the buggy code's close and fclose: + * + * int print(int fd) { + * int rc; + * char buf[128]; + * while ((rc = read(fd, buf, sizeof(buf))) > 0) { + * printf("%.*s", rc); + * } + * close(fd); + * } + * + * int bug() { + * FILE* f = fopen("foo", "r"); + * print(fileno(f)); + * fclose(f); + * } + * + * To make it easier to find this class of bugs, bionic provides a method to + * require that file descriptors are closed by their owners. File descriptors + * can be associated with tags with which they must be closed. This allows + * objects that conceptually own an fd (FILE*, unique_fd, etc.) to use their + * own address at the tag, to enforce that closure of the fd must come as a + * result of their own destruction (fclose, ~unique_fd, etc.) + * + * By default, a file descriptor's tag is 0, and close(fd) is equivalent to + * closing fd with the tag 0. + ****************************************************************************/ + +/**************************************************************************** + * For improved diagnostics, the type of a file descriptors owner can be + * encoded in the most significant byte of the owner tag. Values of 0 and + * 0xff are ignored, which allows for raw pointers to be used as owner tags + * without modification. + ****************************************************************************/ + +#ifdef __cplusplus +#define EXTERN extern "C" +extern "C" +{ +#else +#define EXTERN extern +#endif + +enum android_fdsan_owner_type +{ + /************************************************************************** + * Generic Java or native owners. + * + * Generic Java objects always use 255 as their type, using + * identityHashCode as the value of the tag, leaving bits 33-56 unset. + * Native pointers are sign extended from 48-bits of virtual address space, + * and so can have the MSB set to 255 as well. Use the value of bits 49-56 + * to distinguish between these cases. + **************************************************************************/ + + ANDROID_FDSAN_OWNER_TYPE_GENERIC_00 = 0, + ANDROID_FDSAN_OWNER_TYPE_GENERIC_FF = 255, + + /* FILE* */ + + ANDROID_FDSAN_OWNER_TYPE_FILE = 1, + + /* DIR* */ + + ANDROID_FDSAN_OWNER_TYPE_DIR = 2, + + /* android::base::unique_fd */ + + ANDROID_FDSAN_OWNER_TYPE_UNIQUE_FD = 3, + + /* sqlite-owned file descriptors */ + + ANDROID_FDSAN_OWNER_TYPE_SQLITE = 4, + + /* java.io.FileInputStream */ + + ANDROID_FDSAN_OWNER_TYPE_FILEINPUTSTREAM = 5, + + /* java.io.FileOutputStream */ + + ANDROID_FDSAN_OWNER_TYPE_FILEOUTPUTSTREAM = 6, + + /* java.io.RandomAccessFile */ + + ANDROID_FDSAN_OWNER_TYPE_RANDOMACCESSFILE = 7, + + /* android.os.ParcelFileDescriptor */ + + ANDROID_FDSAN_OWNER_TYPE_PARCELFILEDESCRIPTOR = 8, + + /* ART FdFile */ + + ANDROID_FDSAN_OWNER_TYPE_ART_FDFILE = 9, + + /* java.net.DatagramSocketImpl */ + + ANDROID_FDSAN_OWNER_TYPE_DATAGRAMSOCKETIMPL = 10, + + /* java.net.SocketImpl */ + + ANDROID_FDSAN_OWNER_TYPE_SOCKETIMPL = 11, + + /* libziparchive's ZipArchive */ + + ANDROID_FDSAN_OWNER_TYPE_ZIPARCHIVE = 12, +}; + +typedef enum android_fdsan_owner_type android_fdsan_owner_type_t; + +/**************************************************************************** + * Create an owner tag with the specified type and + * least significant 56 bits of tag. + ****************************************************************************/ + +uint64_t android_fdsan_create_owner_tag(enum android_fdsan_owner_type type, + uint64_t tag); + +/**************************************************************************** + * Exchange a file descriptor's tag. + * + * Logs and aborts if the fd's tag does not match expected_tag. + ****************************************************************************/ + +void android_fdsan_exchange_owner_tag(int fd, uint64_t expected_tag, + uint64_t new_tag); + +/**************************************************************************** + * Close a file descriptor with a tag, and resets the tag to 0. + * + * Logs and aborts if the tag is incorrect. + ****************************************************************************/ + +int android_fdsan_close_with_tag(int fd, uint64_t tag); + +#undef EXTERN +#ifdef __cplusplus +} +#endif + +#endif /* __INCLUDE_ANDROID_FDSAN_H */ diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h index d2db19e987cf5..17410a5ba6fbf 100644 --- a/include/nuttx/fs/fs.h +++ b/include/nuttx/fs/fs.h @@ -430,6 +430,9 @@ struct file off_t f_pos; /* File position */ FAR struct inode *f_inode; /* Driver or file system interface */ FAR void *f_priv; /* Per file driver private data */ +#ifdef CONFIG_FDSAN + uint64_t f_tag; /* file owner tag, init to 0 */ +#endif }; /* This defines a two layer array of files indexed by the file descriptor. diff --git a/include/nuttx/fs/ioctl.h b/include/nuttx/fs/ioctl.h index 8ce62f05d4fde..25bd11c9448f1 100644 --- a/include/nuttx/fs/ioctl.h +++ b/include/nuttx/fs/ioctl.h @@ -184,6 +184,18 @@ * OUT: None */ +#ifdef CONFIG_FDSAN +#define FIOC_SETTAG _FIOC(0x000e) /* IN: FAR uint64_t * + * Pointer to file tag + * OUT: None + */ + +#define FIOC_GETTAG _FIOC(0x000f) /* IN: FAR uint64_t * + * Pointer to file tag + * OUT: None + */ +#endif + /* NuttX file system ioctl definitions **************************************/ #define _DIOCVALID(c) (_IOC_TYPE(c)==_DIOCBASE) diff --git a/libs/libc/dirent/lib_closedir.c b/libs/libc/dirent/lib_closedir.c index e547dc9ad8543..05c4435004ac0 100644 --- a/libs/libc/dirent/lib_closedir.c +++ b/libs/libc/dirent/lib_closedir.c @@ -24,6 +24,11 @@ #include #include + +#ifdef CONFIG_FDSAN +# include +#endif + #include #include "libc.h" @@ -56,6 +61,9 @@ int closedir(FAR DIR *dirp) { int ret; +#ifdef CONFIG_FDSAN + uint64_t tag; +#endif if (dirp == NULL) { @@ -63,7 +71,14 @@ int closedir(FAR DIR *dirp) return -1; } +#ifdef CONFIG_FDSAN + tag = android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_DIR, + (uintptr_t)dirp); + ret = android_fdsan_close_with_tag(dirp->fd, tag); +#else ret = close(dirp->fd); +#endif + lib_free(dirp); return ret; } diff --git a/libs/libc/dirent/lib_opendir.c b/libs/libc/dirent/lib_opendir.c index aba695327734f..3212ff41dee9e 100644 --- a/libs/libc/dirent/lib_opendir.c +++ b/libs/libc/dirent/lib_opendir.c @@ -25,6 +25,11 @@ #include #include #include + +#ifdef CONFIG_FDSAN +# include +#endif + #include #include "libc.h" @@ -83,5 +88,12 @@ FAR DIR *opendir(FAR const char *path) } dir->fd = fd; + +#ifdef CONFIG_FDSAN + android_fdsan_exchange_owner_tag(fd, 0, + android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_DIR, + (uintptr_t)dir)); +#endif + return dir; } diff --git a/libs/libc/misc/Kconfig b/libs/libc/misc/Kconfig index fc7de51b7d889..b034caf01f815 100644 --- a/libs/libc/misc/Kconfig +++ b/libs/libc/misc/Kconfig @@ -62,6 +62,12 @@ config LIBC_ENVPATH Use the contents of the common environment variable to locate executable or library files. Default: n +config FDSAN + bool "Enable Fdsan" + default n + ---help--- + Enable the fdsan support + config LIBC_FTOK_VFS_PATH string "Relative path to ftok storage" default "/var/ftok" diff --git a/libs/libc/misc/Make.defs b/libs/libc/misc/Make.defs index 3a52b25b11903..edf17d3438280 100644 --- a/libs/libc/misc/Make.defs +++ b/libs/libc/misc/Make.defs @@ -60,6 +60,12 @@ ifeq ($(CONFIG_LIBC_ENVPATH),y) CSRCS += lib_envpath.c endif +# Fdsan support + +ifeq ($(CONFIG_FDSAN),y) +CSRCS += lib_fdsan.c +endif + # To ensure uname information is newest, # add lib_utsname.o to phony target for force rebuild diff --git a/libs/libc/misc/lib_fdsan.c b/libs/libc/misc/lib_fdsan.c new file mode 100644 index 0000000000000..dd1ec5a59f8eb --- /dev/null +++ b/libs/libc/misc/lib_fdsan.c @@ -0,0 +1,206 @@ +/**************************************************************************** + * libs/libc/misc/lib_fdsan.c + * Copyright (C) 2018 The Android Open Source Project + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * 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. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS 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 + * COPYRIGHT OWNER 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. + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include + +#include +#include +#include +#include +#include + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +uint64_t android_fdsan_get_tag_value(uint64_t tag) +{ + /* Lop off the most significant byte and sign extend. */ + + return (uint64_t)((int64_t)(tag << 8) >> 8); +} + +const char *android_fdsan_get_tag_type(uint64_t tag) +{ + uint64_t high_bits; + uint64_t type = tag >> 56; + switch (type) + { + case ANDROID_FDSAN_OWNER_TYPE_FILE: + return "FILE*"; + case ANDROID_FDSAN_OWNER_TYPE_DIR: + return "DIR*"; + case ANDROID_FDSAN_OWNER_TYPE_UNIQUE_FD: + return "unique_fd"; + case ANDROID_FDSAN_OWNER_TYPE_FILEINPUTSTREAM: + return "FileInputStream"; + case ANDROID_FDSAN_OWNER_TYPE_FILEOUTPUTSTREAM: + return "FileOutputStream"; + case ANDROID_FDSAN_OWNER_TYPE_RANDOMACCESSFILE: + return "RandomAccessFile"; + case ANDROID_FDSAN_OWNER_TYPE_PARCELFILEDESCRIPTOR: + return "ParcelFileDescriptor"; + case ANDROID_FDSAN_OWNER_TYPE_SQLITE: + return "sqlite"; + case ANDROID_FDSAN_OWNER_TYPE_ART_FDFILE: + return "ART FdFile"; + case ANDROID_FDSAN_OWNER_TYPE_DATAGRAMSOCKETIMPL: + return "DatagramSocketImpl"; + case ANDROID_FDSAN_OWNER_TYPE_SOCKETIMPL: + return "SocketImpl"; + case ANDROID_FDSAN_OWNER_TYPE_ZIPARCHIVE: + return "ZipArchive"; + + case ANDROID_FDSAN_OWNER_TYPE_GENERIC_00: + default: + return "native object of unknown type"; + + case ANDROID_FDSAN_OWNER_TYPE_GENERIC_FF: + + /******************************************************************** + * If bits 48 to 56 are set, + * this is a sign-extended generic native pointer + ********************************************************************/ + + high_bits = tag >> 48; + if (high_bits == (1 << 16) - 1) + { + return "native object of unknown type"; + } + + return "Java object of unknown type"; + } +} + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +uint64_t android_fdsan_create_owner_tag(android_fdsan_owner_type_t type, + uint64_t tag) +{ + uint64_t result; + uint64_t mask; + + if (tag == 0) + { + return 0; + } + + DEBUGASSERT((type & 0xff) == type); + + result = (uint64_t)(type) << 56; + mask = ((uint64_t)1 << 56) - 1; + result |= tag & mask; + return result; +} + +int android_fdsan_close_with_tag(int fd, uint64_t expected_tag) +{ + int ret; + + android_fdsan_exchange_owner_tag(fd, expected_tag, 0); + ret = close(fd); + + /************************************************************************** + * If we were expecting to close with a tag, abort on EBADF. + **************************************************************************/ + + if (expected_tag && ret == -1 && errno == EBADF) + { + ferr("double-close of file descriptor %d detected\n", fd); + PANIC(); + } + + return ret; +} + +void android_fdsan_exchange_owner_tag(int fd, uint64_t expected_tag, + uint64_t new_tag) +{ + uint64_t tag; + int ret; + + ret = ioctl(fd, FIOC_GETTAG, &tag); + if (ret < 0) + { + return; + } + + if (tag == expected_tag) + { + ret = ioctl(fd, FIOC_SETTAG, &new_tag); + DEBUGASSERT(ret == 0); + } + else + { + if (expected_tag && tag) + { + ferr("failed to exchange ownership of file descriptor: fd %d is " + "owned by %s 0x%" PRIx64 ", was expected" + "to be owned by %s 0x%" PRIx64 "\n", + fd, android_fdsan_get_tag_type(tag), + android_fdsan_get_tag_value(tag), + android_fdsan_get_tag_type(expected_tag), + android_fdsan_get_tag_value(expected_tag)); + PANIC(); + } + else if (expected_tag && !tag) + { + ferr("failed to exchange ownership of file descriptor: fd %d is " + "unowned, was expected to be owned by %s 0x%" PRIx64 "\n", + fd, android_fdsan_get_tag_type(expected_tag), + android_fdsan_get_tag_value(expected_tag)); + PANIC(); + } + else if (!expected_tag && tag) + { + ferr("failed to exchange ownership of file descriptor: fd %d is " + "owned by %s 0x%" PRIx64 ", was expected to be unowned\n", + fd, android_fdsan_get_tag_type(tag), + android_fdsan_get_tag_value(tag)); + PANIC(); + } + else if (!expected_tag && !tag) + { + /****************************************************************** + * This should never happen: our CAS failed, + * but expected == actual? + ******************************************************************/ + + ferr("fdsan atomic_compare_exchange_strong failed unexpectedly " + "while exchanging owner tag\n"); + PANIC(); + } + } +} diff --git a/libs/libc/stdio/lib_fclose.c b/libs/libc/stdio/lib_fclose.c index df6cd9ab3cadc..b85a497e7d05f 100644 --- a/libs/libc/stdio/lib_fclose.c +++ b/libs/libc/stdio/lib_fclose.c @@ -30,6 +30,10 @@ #include #include +#ifdef CONFIG_FDSAN +# include +#endif + #include "libc.h" /**************************************************************************** @@ -119,7 +123,14 @@ int fclose(FAR FILE *stream) { /* Close the file descriptor and save the return status */ +#ifdef CONFIG_FDSAN + uint64_t tag; + tag = android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_FILE, + (uintptr_t)stream); + status = android_fdsan_close_with_tag(stream->fs_fd, tag); +#else status = close(stream->fs_fd); +#endif /* If close() returns an error but flush() did not then make sure * that we return the close() error condition. diff --git a/libs/libc/stdio/lib_fopen.c b/libs/libc/stdio/lib_fopen.c index 4de7738bd0118..ae6e851990012 100644 --- a/libs/libc/stdio/lib_fopen.c +++ b/libs/libc/stdio/lib_fopen.c @@ -32,6 +32,10 @@ #include #include +#ifdef CONFIG_FDSAN +# include +#endif + #include "libc.h" /**************************************************************************** @@ -124,6 +128,12 @@ FAR FILE *fopen(FAR const char *path, FAR const char *mode) } } +#ifdef CONFIG_FDSAN + android_fdsan_exchange_owner_tag(fd, 0, + android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_FILE, + (uintptr_t)filep)); +#endif + return filep; }