From 90387a5b41dfefde562cd43a7316ec2fad7f1366 Mon Sep 17 00:00:00 2001 From: hujun5 Date: Mon, 8 May 2023 07:37:45 +0800 Subject: [PATCH] libc/misc: add fdsan module FD (file descriptor) is widely used in system software development, and almost all implementations of posix os (including nuttx) use FD as an index. the value of fd needs to be allocated starting from the minimum available value of 3, and each process has a copy, so the same fd value is very easy to reuse in the program. In multi threaded or multi process environments without address isolation, If the ownership, global variables, and competition relationships of fd are not properly handled, there may be issues with fd duplication or accidental closure. Further leading to the following issues, which are difficult to troubleshoot. 1. Security vulnerability: the fd we wrote is not the expected fd and will be accessed by hackers to obtain data 2. Program exceptions or crashes: write or read fd failures, and program logic errors 3. The structured file XML or database is damaged: the data format written to the database is not the expected format. The implementation principle of fdsan is based on the implementation of Android https://android.googlesource.com/platform/bionic/+/master/docs/fdsan.md Signed-off-by: hujun5 --- fs/inode/fs_files.c | 8 ++ fs/vfs/fs_ioctl.c | 17 +++ include/android/fdsan.h | 190 +++++++++++++++++++++++++++++ include/nuttx/fs/fs.h | 3 + include/nuttx/fs/ioctl.h | 12 ++ libs/libc/dirent/lib_closedir.c | 15 +++ libs/libc/dirent/lib_opendir.c | 12 ++ libs/libc/misc/Kconfig | 6 + libs/libc/misc/Make.defs | 6 + libs/libc/misc/lib_fdsan.c | 206 ++++++++++++++++++++++++++++++++ libs/libc/stdio/lib_fclose.c | 11 ++ libs/libc/stdio/lib_fopen.c | 10 ++ 12 files changed, 496 insertions(+) create mode 100644 include/android/fdsan.h create mode 100644 libs/libc/misc/lib_fdsan.c 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; }