From 577eb47966fc4c92e8aaab97b43f456b210d9c0a Mon Sep 17 00:00:00 2001 From: hujun5 Date: Wed, 8 Nov 2023 18:00:07 +0800 Subject: [PATCH] fdcheck: Enable fdcheck to automatically detect ownership of fd Signed-off-by: hujun5 --- fs/inode/fs_files.c | 6 ++- fs/vfs/fs_dup2.c | 10 ++++ fs/vfs/fs_ioctl.c | 25 ++++++---- include/nuttx/fdcheck.h | 20 ++++---- include/nuttx/fs/fs.h | 6 ++- include/nuttx/fs/ioctl.h | 16 ++++++- libs/libc/misc/lib_fdcheck.c | 89 ++++++++++++++++++++++++++++++------ libs/libc/misc/lib_fdsan.c | 4 +- 8 files changed, 137 insertions(+), 39 deletions(-) diff --git a/fs/inode/fs_files.c b/fs/inode/fs_files.c index c5f047ba94..7018bef995 100644 --- a/fs/inode/fs_files.c +++ b/fs/inode/fs_files.c @@ -254,7 +254,11 @@ static int nx_dup3_from_tcb(FAR struct tcb_s *tcb, int fd1, int fd2, ret = file_dup3(files_fget(list, fd1), filep, flags); #ifdef CONFIG_FDSAN - filep->f_tag = file.f_tag; + filep->f_tag_fdsan = file.f_tag_fdsan; +#endif + +#ifdef CONFIG_FDCHECK + filep->f_tag_fdcheck = file.f_tag_fdcheck; #endif file_close(&file); diff --git a/fs/vfs/fs_dup2.c b/fs/vfs/fs_dup2.c index cdfb9809df..7bf6102851 100644 --- a/fs/vfs/fs_dup2.c +++ b/fs/vfs/fs_dup2.c @@ -163,6 +163,16 @@ int file_dup3(FAR struct file *filep1, FAR struct file *filep2, int flags) ret = file_close(filep2); DEBUGASSERT(ret == 0); + /* Copy tag */ + +#ifdef CONFIG_FDSAN + temp.f_tag_fdsan = filep1->f_tag_fdsan; +#endif + +#ifdef CONFIG_FDCHECK + temp.f_tag_fdcheck = filep1->f_tag_fdcheck; +#endif + /* Return the file structure */ memcpy(filep2, &temp, sizeof(temp)); diff --git a/fs/vfs/fs_ioctl.c b/fs/vfs/fs_ioctl.c index 97e098132f..672a92edcd 100644 --- a/fs/vfs/fs_ioctl.c +++ b/fs/vfs/fs_ioctl.c @@ -43,9 +43,6 @@ static 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; @@ -113,15 +110,25 @@ static 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; + case FIOC_SETTAG_FDSAN: + filep->f_tag_fdsan = *(FAR uint64_t *)arg; ret = OK; break; - case FIOC_GETTAG: - tag = (FAR uint64_t *)arg; - *tag = filep->f_tag; + case FIOC_GETTAG_FDSAN: + *(FAR uint64_t *)arg = filep->f_tag_fdsan; + ret = OK; + break; +#endif + +#ifdef CONFIG_FDCHECK + case FIOC_SETTAG_FDCHECK: + filep->f_tag_fdcheck = *(FAR uint8_t *)arg; + ret = OK; + break; + + case FIOC_GETTAG_FDCHECK: + *(FAR uint8_t *)arg = filep->f_tag_fdcheck; ret = OK; break; #endif diff --git a/include/nuttx/fdcheck.h b/include/nuttx/fdcheck.h index abf7e4bacc..c78f4d3a9d 100644 --- a/include/nuttx/fdcheck.h +++ b/include/nuttx/fdcheck.h @@ -46,21 +46,22 @@ extern "C" * * Description: Obtain original fd information * - * Val carries the pid and fd information. + * Val carries the pid, tag and fd information. * The original fd information is stored in low bit of val. - * The pid information is stored in the high bit of val. + * The pid and tag information is stored in the high bit of val. * For ease of understanding, let's give an example where * the following information is represented in 32-bit binary format * - * val 00000000 00000000 01010101 10001010 + * val 00000000 01010101 00000001 10001010 * fd 00000000 00000000 00000000 10001010 * pid 00000000 00000000 00000000 01010101 + * tag 00000000 00000000 00000000 00000001 * - * In this function, we also check if the pid information is correct. + * In this function, we also check if the pid and tag information is correct. * If there is an error, it will panic. * * Input Parameters: - * val - this val carrying pid and original fd information + * val - this val carrying pid, tag and original fd information * * Returned Value: none * @@ -71,17 +72,18 @@ int fdcheck_restore(int fd); /**************************************************************************** * Name: fdcheck_protect * - * Description: Obtain the combined value of fd and pid + * Description: Obtain the combined value of fd, pid and tag * - * the return value carries the pid and fd information. + * the return value carries the pid, tag and fd information. * The original fd information is stored in low bit of val. - * The pid information is stored in high bit of val. + * The pid and tag information is stored in high bit of val. * For ease of understanding, let's give an example where * the following information is represented in 32-bit binary format * * fd 00000000 00000000 00000000 10001010 * pid 00000000 00000000 00000000 01010101 - * val 00000000 00000000 01010101 10001010 + * tag 00000000 00000000 00000000 00000001 + * val 00000000 01010101 00000001 10001010 * * Input Parameters: * fd - original fd diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h index 3d2ccec42b..b41272f8b1 100644 --- a/include/nuttx/fs/fs.h +++ b/include/nuttx/fs/fs.h @@ -468,7 +468,11 @@ struct file 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 */ + uint64_t f_tag_fdsan; /* File owner fdsan tag, init to 0 */ +#endif + +#ifdef CONFIG_FDCHECK + uint8_t f_tag_fdcheck; /* File owner fdcheck tag, init to 0 */ #endif }; diff --git a/include/nuttx/fs/ioctl.h b/include/nuttx/fs/ioctl.h index 0210e71cae..7942f54926 100644 --- a/include/nuttx/fs/ioctl.h +++ b/include/nuttx/fs/ioctl.h @@ -189,17 +189,29 @@ */ #ifdef CONFIG_FDSAN -#define FIOC_SETTAG _FIOC(0x000e) /* IN: FAR uint64_t * +#define FIOC_SETTAG_FDSAN _FIOC(0x000e) /* IN: FAR uint64_t * * Pointer to file tag * OUT: None */ -#define FIOC_GETTAG _FIOC(0x000f) /* IN: FAR uint64_t * +#define FIOC_GETTAG_FDSAN _FIOC(0x000f) /* IN: FAR uint64_t * * Pointer to file tag * OUT: None */ #endif +#ifdef CONFIG_FDCHECK +#define FIOC_SETTAG_FDCHECK _FIOC(0x0010) /* IN: FAR uint8_t * + * Pointer to file fdcheck tag + * OUT: None + */ + +#define FIOC_GETTAG_FDCHECK _FIOC(0x0011) /* IN: FAR uint8_t * + * Pointer to file fdcheck tag + * OUT: None + */ +#endif + /* NuttX file system ioctl definitions **************************************/ #define _DIOCVALID(c) (_IOC_TYPE(c)==_DIOCBASE) diff --git a/libs/libc/misc/lib_fdcheck.c b/libs/libc/misc/lib_fdcheck.c index 9163faacee..e66e73d269 100644 --- a/libs/libc/misc/lib_fdcheck.c +++ b/libs/libc/misc/lib_fdcheck.c @@ -25,6 +25,8 @@ #include #include #include +#include +#include #include #include @@ -39,10 +41,21 @@ #define FD_BITS LOG2_CEIL(OPEN_MAX) #define FD_MASK ((1 << FD_BITS) - 1) -#define PID_SHIFT (FD_BITS + FD_SHIFT) +#define TAG_SHIFT (FD_BITS + FD_SHIFT) +#define TAG_BITS 8 +#define TAG_MASK ((1 << TAG_BITS) - 1) + +#define PID_SHIFT (TAG_BITS + TAG_SHIFT) #define PID_BITS (8 * sizeof(int) - 1 - PID_SHIFT) #define PID_MASK ((1 << PID_BITS) - 1) +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static spinlock_t g_fdcheck_lock = SP_UNLOCKED; +static uint8_t g_fdcheck_tag = 0; + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -52,21 +65,22 @@ * * Description: Obtain original fd information * - * Val carries the pid and fd information. + * Val carries the pid, tag and fd information. * The original fd information is stored in low bit of val. - * The pid information is stored in the high bit of val. + * The pid and tag information is stored in the high bit of val. * For ease of understanding, let's give an example where * the following information is represented in 32-bit binary format * - * val 00000000 00000000 01010101 10001010 + * val 00000000 01010101 00000001 10001010 * fd 00000000 00000000 00000000 10001010 * pid 00000000 00000000 00000000 01010101 + * tag 00000000 00000000 00000000 00000001 * - * In this function, we also check if the pid information is correct. + * In this function, we also check if the pid and tag information is correct. * If there is an error, it will panic. * * Input Parameters: - * val - this val carrying pid and original fd information + * val - this val carrying pid, tag and original fd information * * Returned Value: none * @@ -75,17 +89,17 @@ int fdcheck_restore(int val) { int pid_expect; - int pid_now; int ppid_now; + int pid_now; if (val <= 2) { return val; } - pid_expect = (val >> PID_SHIFT); - pid_now = (_SCHED_GETPID() & PID_MASK); - ppid_now = (_SCHED_GETPPID() & PID_MASK); + pid_expect = (val >> PID_SHIFT) & PID_MASK; + pid_now = _SCHED_GETPID() & PID_MASK; + ppid_now = _SCHED_GETPPID() & PID_MASK; if (pid_expect != pid_now && pid_expect != ppid_now && pid_expect != 0) { ferr("pid_expect %d pid_now %d ppid_now %d\n", @@ -93,23 +107,40 @@ int fdcheck_restore(int val) PANIC(); } + if (pid_expect != 0) + { + uint8_t tag_store; + int ret = ioctl(val & FD_MASK, FIOC_GETTAG_FDCHECK, &tag_store); + if (ret >= 0) + { + uint8_t tag_expect = (val >> TAG_SHIFT) & TAG_MASK; + if (tag_expect != tag_store) + { + ferr("tag_expect 0x%x tag_store 0x%x\n", + tag_expect, tag_store); + PANIC(); + } + } + } + return val & FD_MASK; } /**************************************************************************** * Name: fdcheck_protect * - * Description: Obtain the combined value of fd and pid + * Description: Obtain the combined value of fd, pid and tag * - * the return value carries the pid and fd information. + * the return value carries the pid, tag and fd information. * The original fd information is stored in low bit of val. - * The pid information is stored in high bit of val. + * The pid and tag information is stored in high bit of val. * For ease of understanding, let's give an example where * the following information is represented in 32-bit binary format * * fd 00000000 00000000 00000000 10001010 * pid 00000000 00000000 00000000 01010101 - * val 00000000 00000000 01010101 10001010 + * tag 00000000 00000000 00000000 00000001 + * val 00000000 01010101 00000001 10001010 * * Input Parameters: * fd - original fd @@ -120,12 +151,40 @@ int fdcheck_restore(int val) int fdcheck_protect(int fd) { + int protect_fd; + uint8_t tag; + int ret; + if (fd <= 2) { return fd; } - return (fd & FD_MASK) | ((_SCHED_GETPID() & PID_MASK) << PID_SHIFT); + protect_fd = fd & FD_MASK; + protect_fd |= (_SCHED_GETPID() & PID_MASK) << PID_SHIFT; + + ret = ioctl(fd, FIOC_GETTAG_FDCHECK, &tag); + DEBUGASSERT(ret >= 0); + if (tag == 0) + { + irqstate_t flags = spin_lock_irqsave(&g_fdcheck_lock); + if ((++g_fdcheck_tag & TAG_MASK) == 0) + { + ++g_fdcheck_tag; + } + + g_fdcheck_tag &= TAG_MASK; + protect_fd |= g_fdcheck_tag << TAG_SHIFT; + ret = ioctl(fd, FIOC_SETTAG_FDCHECK, &g_fdcheck_tag); + DEBUGASSERT(ret == 0); + spin_unlock_irqrestore(&g_fdcheck_lock, flags); + } + else + { + protect_fd |= (tag & TAG_MASK) << TAG_SHIFT; + } + + return protect_fd; } #endif diff --git a/libs/libc/misc/lib_fdsan.c b/libs/libc/misc/lib_fdsan.c index b71b2ac4c4..628aa652cb 100644 --- a/libs/libc/misc/lib_fdsan.c +++ b/libs/libc/misc/lib_fdsan.c @@ -151,7 +151,7 @@ void android_fdsan_exchange_owner_tag(int fd, uint64_t expected_tag, uint64_t tag; int ret; - ret = ioctl(fd, FIOC_GETTAG, &tag); + ret = ioctl(fd, FIOC_GETTAG_FDSAN, &tag); if (ret < 0) { return; @@ -159,7 +159,7 @@ void android_fdsan_exchange_owner_tag(int fd, uint64_t expected_tag, if (tag == expected_tag) { - ret = ioctl(fd, FIOC_SETTAG, &new_tag); + ret = ioctl(fd, FIOC_SETTAG_FDSAN, &new_tag); DEBUGASSERT(ret == 0); } else