From 9fb7b0358b2573e1db1a44b3dd0520f1c9e231a2 Mon Sep 17 00:00:00 2001 From: hujun5 Date: Wed, 10 Apr 2024 09:23:43 +0800 Subject: [PATCH] fdcheck: update fdcheck impl 1 store fd in the high position 2 removing the pid information , as the tag information is sufficient. Signed-off-by: hujun5 --- include/nuttx/fdcheck.h | 28 +++++---- libs/libc/misc/lib_fdcheck.c | 107 +++++++---------------------------- libs/libc/stdio/lib_fopen.c | 12 ++-- 3 files changed, 40 insertions(+), 107 deletions(-) diff --git a/include/nuttx/fdcheck.h b/include/nuttx/fdcheck.h index c78f4d3a9ded8..af55f87ed1c5c 100644 --- a/include/nuttx/fdcheck.h +++ b/include/nuttx/fdcheck.h @@ -46,49 +46,47 @@ extern "C" * * Description: Obtain original fd information * - * Val carries the pid, tag and fd information. - * The original fd information is stored in low bit of val. - * The pid and tag information is stored in the high bit of val. + * Val carries the tag and fd information. + * The original fd information is stored in high bit of val. + * The tag information is stored in the low 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 01010101 00000001 10001010 + * val 00000000 00000000 10001010 00000001 * fd 00000000 00000000 00000000 10001010 - * pid 00000000 00000000 00000000 01010101 * tag 00000000 00000000 00000000 00000001 * - * In this function, we also check if the pid and tag information is correct. + * In this function, we also check tag information is correct. * If there is an error, it will panic. * * Input Parameters: - * val - this val carrying pid, tag and original fd information + * val - this val carrying tag and original fd information * - * Returned Value: none + * Returned Value: The original fd is returned. * ****************************************************************************/ -int fdcheck_restore(int fd); +int fdcheck_restore(int val); /**************************************************************************** * Name: fdcheck_protect * - * Description: Obtain the combined value of fd, pid and tag + * Description: Obtain the combined value of fd and tag * - * the return value carries the pid, tag and fd information. + * the return value carries the tag and fd information. * The original fd information is stored in low bit of val. - * The pid and tag information is stored in high bit of val. + * The 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 * tag 00000000 00000000 00000000 00000001 - * val 00000000 01010101 00000001 10001010 + * val 00000000 00000000 10001010 00000001 * * Input Parameters: * fd - original fd * - * Returned Value: the combined value of fd and pid + * Returned Value: the combined value of fd and tag * ****************************************************************************/ diff --git a/libs/libc/misc/lib_fdcheck.c b/libs/libc/misc/lib_fdcheck.c index e66e73d269518..8cf56dcc3b62a 100644 --- a/libs/libc/misc/lib_fdcheck.c +++ b/libs/libc/misc/lib_fdcheck.c @@ -37,17 +37,15 @@ * Pre-processor Definitions ****************************************************************************/ -#define FD_SHIFT 0 -#define FD_BITS LOG2_CEIL(OPEN_MAX) -#define FD_MASK ((1 << FD_BITS) - 1) - -#define TAG_SHIFT (FD_BITS + FD_SHIFT) +#define TAG_SHIFT 0 #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) +#define FD_SHIFT (TAG_SHIFT + TAG_BITS) +#define FD_BITS LOG2_CEIL(OPEN_MAX) +#define FD_MASK ((1 << FD_BITS) - 1) + +static_assert(FD_BITS <= TAG_BITS, "FD_BITS is too long"); /**************************************************************************** * Private Data @@ -60,95 +58,34 @@ static uint8_t g_fdcheck_tag = 0; * Public Functions ****************************************************************************/ -/**************************************************************************** - * Name: fdcheck_restore - * - * Description: Obtain original fd information - * - * Val carries the pid, tag and fd information. - * The original fd information is stored in low 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 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 and tag information is correct. - * If there is an error, it will panic. - * - * Input Parameters: - * val - this val carrying pid, tag and original fd information - * - * Returned Value: none - * - ****************************************************************************/ - int fdcheck_restore(int val) { - int pid_expect; - int ppid_now; - int pid_now; + uint8_t tag_store; + int fd; - if (val <= 2) - { - return val; - } + /* If val is a bare fd（0~255）, we should return it directly */ - 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) + fd = (val >> FD_SHIFT) & FD_MASK; + if (fd == 0 || val < 0) { - ferr("pid_expect %d pid_now %d ppid_now %d\n", - pid_expect, pid_now, ppid_now); - PANIC(); + return val; } - if (pid_expect != 0) + int ret = ioctl(fd, FIOC_GETTAG_FDCHECK, &tag_store); + if (ret >= 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) { - 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(); - } + ferr("tag_expect 0x%x tag_store 0x%x\n", + tag_expect, tag_store); + PANIC(); } } - return val & FD_MASK; + return fd; } -/**************************************************************************** - * Name: fdcheck_protect - * - * Description: Obtain the combined value of fd, pid and tag - * - * the return value carries the pid, tag and fd information. - * The original fd information is stored in low 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 - * tag 00000000 00000000 00000000 00000001 - * val 00000000 01010101 00000001 10001010 - * - * Input Parameters: - * fd - original fd - * - * Returned Value: the combined value of fd and pid - * - ****************************************************************************/ - int fdcheck_protect(int fd) { int protect_fd; @@ -160,9 +97,7 @@ int fdcheck_protect(int fd) return fd; } - protect_fd = fd & FD_MASK; - protect_fd |= (_SCHED_GETPID() & PID_MASK) << PID_SHIFT; - + protect_fd = (fd & FD_MASK) << FD_SHIFT; ret = ioctl(fd, FIOC_GETTAG_FDCHECK, &tag); DEBUGASSERT(ret >= 0); if (tag == 0) diff --git a/libs/libc/stdio/lib_fopen.c b/libs/libc/stdio/lib_fopen.c index c0283feb56f9a..1576ee41f7560 100644 --- a/libs/libc/stdio/lib_fopen.c +++ b/libs/libc/stdio/lib_fopen.c @@ -105,6 +105,12 @@ FAR FILE *fdopen(int fd, FAR const char *mode) /* Initialize the mutex the manages access to the buffer */ nxrmutex_init(&filep->fs_lock); + +#ifdef CONFIG_FDSAN + android_fdsan_exchange_owner_tag(fd, 0, + android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_FILE, + (uintptr_t)filep)); +#endif } else { @@ -135,12 +141,6 @@ FAR FILE *fdopen(int fd, FAR const char *mode) filep->fs_cookie = (FAR void *)(intptr_t)fd; filep->fs_oflags = oflags; -#ifdef CONFIG_FDSAN - android_fdsan_exchange_owner_tag(fd, 0, - android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_FILE, - (uintptr_t)filep)); -#endif - /* Assign custom callbacks to NULL. */ filep->fs_iofunc.read = NULL;