This is an automated email from the ASF dual-hosted git repository.
xiaoxiang781216 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 3024b27e442 sched: guard task-exit recovery against corrupted user
lists.
3024b27e442 is described below
commit 3024b27e442f7541eda1c01a8353adc44f7a4be4
Author: Marco Casaroli <[email protected]>
AuthorDate: Mon Oct 26 19:57:06 2026 +0100
sched: guard task-exit recovery against corrupted user lists.
In kernel and protected builds the TLS block and the robust-mutex list it
anchors live in user memory. At task exit nxtask_recover() walked
tls->tl_mhead and wrote through each node (mutex_reset(), flink and flags)
in kernel mode, and task_uninit_info() likewise freed user-heap nodes. A
task whose TLS or list is corrupted, for example by a stack overflow, made
the kernel fault or follow a user-controlled pointer while privileged.
Validate every list node before it is dereferenced:
- pthread_mutex_inconsistent() moves from libs/libc/pthread to
sched/pthread: only nxtask_recover() calls it, and the checks below need
the TCB, which user space cannot use.
- pthread_mutex_inconsistent() takes the target tcb and checks that the
TLS and each mutex lie in that task's user memory (kernel builds also
require a mapped backing page and exclude the text region; protected
builds accept only user data, bss and heap), that nodes are aligned, and
that each mutex is robust and owned by the exiting thread; a bad node
ends the walk.
- nxtask_recover() selects the victim's address environment around recovery
so the TLS and an unmapped waitobj are read in the right space, and
restores it even when selection fails.
- task_uninit_info() skips traversing and freeing unprivileged user
task-info allocations in kernel builds; the address environment reclaims
the user heap, and group teardown still releases file descriptors.
- Implement up_addrenv_find_page() for armv7-a and x86_64: the page check
calls it in every kernel build, and these two had none.
Flat builds behave as before; the guards compile out.
Tested: rv-virt:knetnsh64 (kernel): a robust-mutex test passes (owner
death gives EOWNERDEAD; invalid, unmapped and cyclic lists do not panic;
the exit of another process leaves the parent's mutex and semaphore
alone), a program that overflows its stack and corrupts its TLS ends in
SIGSEGV and NSH continues, and ostest exits 0. rv-virt:pnsh64
(protected) and rv-virt:nsh64 (flat): ostest exits 0. qemu-armv7a:knsh:
a robust-mutex exit test passes and ostest exits 0. Builds (Make):
qemu-intel64 knsh_romfs and knsh_romfs_pci, qemu-armv8a:knsh,
rv-virt:knsh64, sabre-6quad:knsh, sama5d4-ek:knsh, beaglebone:knsh,
esp32s3-devkit:kernel_oct (kernel); qemu-armv7r:pnsh, mps2-an500:knsh,
stm32f4discovery:kostest, esp32s3-devkit:knsh, esp32-devkitc:knsh,
fvp-armv8r:pnsh (protected); rv-virt:nsh64 with PTHREAD_MUTEX_UNSAFE.
qemu-armv8a:nsh with CMake.
Assisted-by: Codex:gpt-6.1-sol
Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Marco Casaroli <[email protected]>
---
arch/arm/src/armv7-a/arm_addrenv_pgmap.c | 33 ++++-
arch/x86_64/src/common/x86_64_addrenv.c | 54 +++++++
libs/libc/pthread/CMakeLists.txt | 2 +-
libs/libc/pthread/Make.defs | 2 +-
libs/libc/pthread/pthread_mutex_inconsistent.c | 82 -----------
sched/pthread/CMakeLists.txt | 4 +
sched/pthread/Make.defs | 4 +
sched/pthread/pthread.h | 2 +-
sched/pthread/pthread_mutexinconsistent.c | 187 +++++++++++++++++++++++++
sched/task/task_recover.c | 27 +++-
sched/tls/task_uninitinfo.c | 12 ++
11 files changed, 321 insertions(+), 88 deletions(-)
diff --git a/arch/arm/src/armv7-a/arm_addrenv_pgmap.c
b/arch/arm/src/armv7-a/arm_addrenv_pgmap.c
index e889da1adde..5932049dbd5 100644
--- a/arch/arm/src/armv7-a/arm_addrenv_pgmap.c
+++ b/arch/arm/src/armv7-a/arm_addrenv_pgmap.c
@@ -31,6 +31,7 @@
#include <nuttx/pgalloc.h>
#include "pgalloc.h"
+#include "mmu.h"
/****************************************************************************
* Public Functions
@@ -54,7 +55,36 @@
uintptr_t up_addrenv_find_page(arch_addrenv_t *addrenv, uintptr_t vaddr)
{
- /* Implement it later */
+ uintptr_t entry;
+ uintptr_t l2table;
+
+ if (!arm_uservaddr(vaddr) || addrenv->l1table == NULL)
+ {
+ return 0;
+ }
+
+ entry = mmu_l1table_getentry(addrenv->l1table, vaddr);
+ if ((entry & PMD_TYPE_MASK) != PMD_TYPE_PTE)
+ {
+ return 0;
+ }
+
+ l2table = arm_pgvaddr(entry & PMD_PTE_PADDR_MASK);
+ if (l2table == 0)
+ {
+ return 0;
+ }
+
+ entry = mmu_l2_getentry(l2table, vaddr);
+ if ((entry & PTE_TYPE_SMALL) != 0)
+ {
+ return entry & PTE_SMALL_PADDR_MASK;
+ }
+
+ if ((entry & PTE_TYPE_MASK) == PTE_TYPE_LARGE)
+ {
+ return (entry & PTE_LARGE_PADDR_MASK) | (vaddr & 0xf000);
+ }
return 0;
}
@@ -115,5 +145,6 @@ uintptr_t up_addrenv_page_vaddr(uintptr_t page)
void up_addrenv_page_wipe(uintptr_t page)
{
uintptr_t vaddr = arm_pgvaddr(page);
+
memset((void *)vaddr, 0, MM_PGSIZE);
}
diff --git a/arch/x86_64/src/common/x86_64_addrenv.c
b/arch/x86_64/src/common/x86_64_addrenv.c
index de6296af984..81fd7e0bbcd 100644
--- a/arch/x86_64/src/common/x86_64_addrenv.c
+++ b/arch/x86_64/src/common/x86_64_addrenv.c
@@ -995,6 +995,60 @@ int up_addrenv_detach(struct tcb_s *tcb)
return OK;
}
+/****************************************************************************
+ * Name: up_addrenv_find_page
+ *
+ * Description:
+ * Find physical page mapped to user virtual address from the address
+ * environment page directory.
+ *
+ * Input Parameters:
+ * addrenv - The user address environment.
+ * vaddr - The user virtual address
+ *
+ * Returned Value:
+ * Page physical address on success; NULL on failure.
+ *
+ ****************************************************************************/
+
+uintptr_t up_addrenv_find_page(arch_addrenv_t *addrenv, uintptr_t vaddr)
+{
+ uintptr_t lnvaddr;
+ uintptr_t paddr;
+ uint32_t ptlevel;
+
+ /* If vaddr is not user space, get out */
+
+ if (!up_addrenv_user_vaddr(vaddr))
+ {
+ return 0;
+ }
+
+ /* Walk the tables as the MMU does, from the PML4 (level 0) to the PT
+ * (level 3). An entry that is not used is zero.
+ */
+
+ lnvaddr = x86_64_pgvaddr(addrenv->spgtables[0]);
+ if (!lnvaddr)
+ {
+ return 0;
+ }
+
+ for (ptlevel = 0; ptlevel < X86_MMU_PT_LEVELS - 1; ptlevel++)
+ {
+ paddr = mmu_pte_to_paddr(mmu_ln_getentry(ptlevel, lnvaddr, vaddr));
+ lnvaddr = x86_64_pgvaddr(paddr);
+ if (!lnvaddr)
+ {
+ return 0;
+ }
+ }
+
+ /* Got it, now convert it to physical page */
+
+ return mmu_pte_to_paddr(mmu_ln_getentry(ptlevel, lnvaddr, vaddr));
+}
+
/****************************************************************************
* Name: up_addrenv_user_vaddr
*
diff --git a/libs/libc/pthread/CMakeLists.txt b/libs/libc/pthread/CMakeLists.txt
index 752b35a69e0..b9d4288dae4 100644
--- a/libs/libc/pthread/CMakeLists.txt
+++ b/libs/libc/pthread/CMakeLists.txt
@@ -121,7 +121,7 @@ if(NOT CONFIG_DISABLE_PTHREAD)
thrd_exit.c)
if(NOT CONFIG_PTHREAD_MUTEX_UNSAFE)
- list(APPEND SRCS pthread_mutex_consistent.c pthread_mutex_inconsistent.c)
+ list(APPEND SRCS pthread_mutex_consistent.c)
endif()
if(CONFIG_SMP)
diff --git a/libs/libc/pthread/Make.defs b/libs/libc/pthread/Make.defs
index 876eb5ffd1f..129cd8db847 100644
--- a/libs/libc/pthread/Make.defs
+++ b/libs/libc/pthread/Make.defs
@@ -76,7 +76,7 @@ CSRCS += pthread_attr_getaffinity.c pthread_attr_setaffinity.c
endif
ifneq ($(CONFIG_PTHREAD_MUTEX_UNSAFE),y)
-CSRCS += pthread_mutex_consistent.c pthread_mutex_inconsistent.c
+CSRCS += pthread_mutex_consistent.c
endif
ifeq ($(CONFIG_PTHREAD_SPINLOCKS),y)
diff --git a/libs/libc/pthread/pthread_mutex_inconsistent.c
b/libs/libc/pthread/pthread_mutex_inconsistent.c
deleted file mode 100644
index d35a1914800..00000000000
--- a/libs/libc/pthread/pthread_mutex_inconsistent.c
+++ /dev/null
@@ -1,82 +0,0 @@
-/****************************************************************************
- * libs/libc/pthread/pthread_mutex_inconsistent.c
- *
- * SPDX-License-Identifier: Apache-2.0
- *
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership. The
- * ASF licenses this file to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance with the
- * License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations
- * under the License.
- *
- ****************************************************************************/
-
-/****************************************************************************
- * Included Files
- ****************************************************************************/
-
-#include <nuttx/config.h>
-
-#include <pthread.h>
-#include <sched.h>
-#include <assert.h>
-#include <errno.h>
-
-#include <nuttx/sched.h>
-#include <nuttx/semaphore.h>
-#include <nuttx/pthread.h>
-#include <nuttx/atomic.h>
-
-#include "pthread.h"
-
-/****************************************************************************
- * Public Functions
- ****************************************************************************/
-
-/****************************************************************************
- * Name: pthread_mutex_inconsistent
- *
- * Description:
- * This function is called when a pthread is terminated via either
- * pthread_exit() or pthread_cancel(). It will check for any mutexes
- * held by exiting thread. It will mark them as inconsistent and
- * then wake up the highest priority waiter for the mutex. That
- * instance of pthread_mutex_lock() will then return EOWNERDEAD.
- *
- * Input Parameters:
- * tcb -- a reference to the TCB of the exiting pthread.
- *
- * Returned Value:
- * None.
- *
- ****************************************************************************/
-
-void pthread_mutex_inconsistent(FAR struct tls_info_s *tls)
-{
- FAR struct pthread_mutex_s *mutex;
-
- /* Remove and process each mutex held by this task */
-
- while (tls->tl_mhead != NULL)
- {
- /* Remove the mutex from the TCB list */
-
- mutex = tls->tl_mhead;
- tls->tl_mhead = mutex->flink;
- mutex->flink = NULL;
-
- /* Mark the mutex as INCONSISTENT and wake up any waiting thread */
-
- mutex->flags |= _PTHREAD_MFLAGS_INCONSISTENT;
- mutex_reset(&mutex->mutex);
- }
-}
diff --git a/sched/pthread/CMakeLists.txt b/sched/pthread/CMakeLists.txt
index 3ce9bb09c11..284fa9295a7 100644
--- a/sched/pthread/CMakeLists.txt
+++ b/sched/pthread/CMakeLists.txt
@@ -37,6 +37,10 @@ if(NOT CONFIG_DISABLE_PTHREAD)
pthread_release.c
pthread_setschedprio.c)
+ if(NOT CONFIG_PTHREAD_MUTEX_UNSAFE)
+ list(APPEND SRCS pthread_mutexinconsistent.c)
+ endif()
+
if(CONFIG_SMP)
list(APPEND SRCS pthread_setaffinity.c pthread_getaffinity.c)
endif()
diff --git a/sched/pthread/Make.defs b/sched/pthread/Make.defs
index 8e28702ac1a..be49d6e5152 100644
--- a/sched/pthread/Make.defs
+++ b/sched/pthread/Make.defs
@@ -28,6 +28,10 @@ CSRCS += pthread_sigmask.c pthread_sigqueue.c
pthread_cancel.c
CSRCS += pthread_completejoin.c pthread_findjoininfo.c
CSRCS += pthread_release.c pthread_setschedprio.c
+ifneq ($(CONFIG_PTHREAD_MUTEX_UNSAFE),y)
+CSRCS += pthread_mutexinconsistent.c
+endif
+
ifeq ($(CONFIG_SMP),y)
CSRCS += pthread_setaffinity.c pthread_getaffinity.c
endif
diff --git a/sched/pthread/pthread.h b/sched/pthread/pthread.h
index 7d8e10e1128..551945b939c 100644
--- a/sched/pthread/pthread.h
+++ b/sched/pthread/pthread.h
@@ -71,7 +71,7 @@ int pthread_findjoininfo(FAR struct task_group_s *group,
pid_t pid,
void pthread_release(FAR struct task_group_s *group);
#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE
-void pthread_mutex_inconsistent(FAR struct tls_info_s *tls);
+void pthread_mutex_inconsistent(FAR struct tcb_s *tcb);
#endif
#ifdef CONFIG_PTHREAD_MUTEX_TYPES
diff --git a/sched/pthread/pthread_mutexinconsistent.c
b/sched/pthread/pthread_mutexinconsistent.c
new file mode 100644
index 00000000000..442d6f726fb
--- /dev/null
+++ b/sched/pthread/pthread_mutexinconsistent.c
@@ -0,0 +1,187 @@
+/****************************************************************************
+ * sched/pthread/pthread_mutexinconsistent.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <pthread.h>
+#include <sched.h>
+#include <assert.h>
+#include <errno.h>
+
+#include <nuttx/sched.h>
+#include <nuttx/semaphore.h>
+#include <nuttx/pthread.h>
+#include <nuttx/atomic.h>
+
+#ifndef CONFIG_BUILD_FLAT
+# include <nuttx/addrenv.h>
+# include <nuttx/arch.h>
+# include <nuttx/mm/mm.h>
+# include <nuttx/pgalloc.h>
+# include <nuttx/userspace.h>
+#endif
+
+#include "pthread/pthread.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifdef CONFIG_PTHREAD_MUTEX_TYPES
+# define mutex_get_holder(m) nxrmutex_get_holder(m)
+# define mutex_reset(m) nxrmutex_reset(m)
+#else
+# define mutex_get_holder(m) nxmutex_get_holder(m)
+# define mutex_reset(m) nxmutex_reset(m)
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#ifndef CONFIG_BUILD_FLAT
+static bool pthread_mutex_accessible(FAR struct tcb_s *tcb,
+ FAR const void *ptr, size_t size)
+{
+ uintptr_t start = (uintptr_t)ptr;
+ uintptr_t end = start + size - 1;
+
+ if ((tcb->flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_KERNEL)
+ {
+ return true;
+ }
+
+ if (start == 0 || end < start)
+ {
+ return false;
+ }
+
+#ifdef CONFIG_BUILD_KERNEL
+ if (!uaccess_ok(ptr, size) || tcb->addrenv_own == NULL ||
+ (start < ARCH_TEXT_VEND && end >= CONFIG_ARCH_TEXT_VBASE))
+ {
+ return false;
+ }
+
+ /* A user virtual address need not have a backing page. In particular,
+ * do not fault again while recovering a task which has already faulted.
+ */
+
+ start = MM_PGALIGNDOWN(start);
+ end = MM_PGALIGNDOWN(end);
+ for (; ; start += MM_PGSIZE)
+ {
+ if (up_addrenv_find_page(&tcb->addrenv_own->addrenv, start) == 0)
+ {
+ return false;
+ }
+
+ if (start == end)
+ {
+ return true;
+ }
+ }
+#else
+ /* Protected builds share an address space, but user lists must still
+ * not lead recovery into kernel memory.
+ */
+
+ return (start >= USERSPACE->us_datastart &&
+ end < USERSPACE->us_dataend) ||
+ (start >= USERSPACE->us_bssstart &&
+ end < USERSPACE->us_bssend) ||
+ (umm_heapmember((FAR void *)start) &&
+ umm_heapmember((FAR void *)end));
+#endif
+}
+#endif
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pthread_mutex_inconsistent
+ *
+ * Description:
+ * This function is called when a pthread is terminated via either
+ * pthread_exit() or pthread_cancel(). It will check for any mutexes
+ * held by exiting thread. It will mark them as inconsistent and
+ * then wake up the highest priority waiter for the mutex. That
+ * instance of pthread_mutex_lock() will then return EOWNERDEAD.
+ *
+ * Input Parameters:
+ * tcb -- a reference to the TCB of the exiting pthread.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+
+void pthread_mutex_inconsistent(FAR struct tcb_s *tcb)
+{
+ FAR struct tls_info_s *tls = tcb->stack_alloc_ptr;
+ FAR struct pthread_mutex_s *mutex;
+
+#ifndef CONFIG_BUILD_FLAT
+ if (!pthread_mutex_accessible(tcb, tls, sizeof(*tls)))
+ {
+ return;
+ }
+#endif
+
+ /* Remove and process each mutex held by this task */
+
+ while (tls->tl_mhead != NULL)
+ {
+ /* Remove the mutex from the TCB list */
+
+ mutex = tls->tl_mhead;
+#ifndef CONFIG_BUILD_FLAT
+ if (((uintptr_t)mutex & (sizeof(uintptr_t) - 1)) != 0 ||
+ !pthread_mutex_accessible(tcb, mutex, sizeof(*mutex)))
+ {
+ tls->tl_mhead = NULL;
+ break;
+ }
+#endif
+
+ if ((mutex->flags & _PTHREAD_MFLAGS_ROBUST) == 0 ||
+ mutex_get_holder(&mutex->mutex) != tcb->pid)
+ {
+ tls->tl_mhead = NULL;
+ break;
+ }
+
+ tls->tl_mhead = mutex->flink;
+ mutex->flink = NULL;
+
+ /* Mark the mutex as INCONSISTENT and wake up any waiting thread */
+
+ mutex->flags |= _PTHREAD_MFLAGS_INCONSISTENT;
+ mutex_reset(&mutex->mutex);
+ }
+}
diff --git a/sched/task/task_recover.c b/sched/task/task_recover.c
index ecdbb9d2dbf..58415db7f41 100644
--- a/sched/task/task_recover.c
+++ b/sched/task/task_recover.c
@@ -29,6 +29,7 @@
#include <assert.h>
#include <nuttx/arch.h>
+#include <nuttx/addrenv.h>
#include <nuttx/wdog.h>
#include <nuttx/sched.h>
@@ -64,10 +65,25 @@
void nxtask_recover(FAR struct tcb_s *tcb)
{
+#ifdef CONFIG_ARCH_ADDRENV
+ FAR struct addrenv_s *oldenv = NULL;
+
+ /* A task may be deleted by a task in another address environment. TLS
+ * and an unmapped waitobj must be accessed in the victim's environment.
+ */
+
+ if (tcb->addrenv_own != NULL &&
+ addrenv_select(tcb->addrenv_own, &oldenv) < 0)
+ {
+ addrenv_restore(oldenv);
+ return;
+ }
+#endif
+
#if !defined(CONFIG_DISABLE_PTHREAD) && !defined(CONFIG_PTHREAD_MUTEX_UNSAFE)
- /* Recover any mutexes still held by the canceled thread */
+ /* Recover any mutexes still held by the canceled thread. */
- pthread_mutex_inconsistent(nxsched_get_tls(tcb));
+ pthread_mutex_inconsistent(tcb);
#endif
/* The task is being deleted. Cancel in pending timeout events. */
@@ -94,4 +110,11 @@ void nxtask_recover(FAR struct tcb_s *tcb)
DEBUGVERIFY(nxsched_stop_sporadic(tcb));
}
#endif
+
+#ifdef CONFIG_ARCH_ADDRENV
+ if (tcb->addrenv_own != NULL)
+ {
+ addrenv_restore(oldenv);
+ }
+#endif
}
diff --git a/sched/tls/task_uninitinfo.c b/sched/tls/task_uninitinfo.c
index b83f2f2f151..9537722b1a8 100644
--- a/sched/tls/task_uninitinfo.c
+++ b/sched/tls/task_uninitinfo.c
@@ -125,6 +125,18 @@ void task_uninit_info(FAR struct task_group_s *group)
{
FAR struct task_info_s *info = group->tg_info;
+#ifdef CONFIG_BUILD_KERNEL
+ /* The user heap is reclaimed with the address environment. Its lists
+ * may have been corrupted by the exiting task; neither traversing them
+ * nor freeing their user allocations is necessary in the kernel.
+ */
+
+ if ((group->tg_flags & GROUP_FLAG_PRIVILEGED) == 0)
+ {
+ return;
+ }
+#endif
+
#ifdef CONFIG_PTHREAD_ATFORK
/* Remove the functions that registered with pthread_atfork() */
