casaroli opened a new pull request, #19544:
URL: https://github.com/apache/nuttx/pull/19544
## Summary
`addrenv_take()` and `addrenv_give()` dereference their argument
unconditionally, but a task does not necessarily own an address environment.
`tcb->addrenv_own` is set only by `addrenv_attach()`, which is reached only
from `addrenv_allocate()`. A kernel thread never allocates one, and in a
protected build nothing does — there is a single address space for the whole
system and the architecture's `up_addrenv_*()` are stubs, so `addrenv_own` is
NULL for every task, always.
`addrenv_join()` calls `addrenv_take(ptcb->addrenv_own)` without a check, so
`pthread_create()` faults on `&((struct addrenv_s *)NULL)->refs` whenever the
calling task has no address environment. With `CONFIG_DEBUG_ASSERTIONS` off
the same access silently corrupts low memory instead.
That a task may have no address environment is already an expected state
everywhere else. In `sched/addrenv/addrenv.c` itself, `addrenv_switch()`
returns `OK` when `tcb->addrenv_curr` is NULL and `addrenv_drop()` returns
early. `addrenv_select()` also calls `addrenv_take()` unconditionally, which
is
exactly why every one of its callers checks first:
| Site | Guard |
|---|---|
| `sched/sched/sched_get_stateinfo.c:91` | `if (tcb->addrenv_own)` |
| `sched/task/task_argvstr.c:75` | `if (tcb->addrenv_own != NULL)` |
| `fs/procfs/fs_procfsproc.c:1494` | `if (info->tcb->addrenv_own != NULL)` |
| `fs/procfs/fs_procfsproc.c:1563` | `if (tcb->addrenv_own != NULL)` |
| `arch/arm/src/common/arm_checkstack.c:235` | `if (tcb->addrenv_own !=
NULL)` |
| `arch/arm64/src/common/arm64_checkstack.c:220` | `if (tcb->addrenv_own !=
NULL)` |
| `arch/risc-v/src/common/riscv_checkstack.c:190` | `if (tcb->addrenv_own !=
NULL)` |
| `arch/tricore/src/common/tricore_checkstack.c:127` | `if (tcb->addrenv_own
!= NULL)` |
`addrenv_join()` is the one path into `addrenv_take()` that cannot push the
check to its caller, and it is the one that was missed. This patch handles
NULL in `addrenv_take()` and `addrenv_give()`, the way the rest of the file
already does.
`addrenv_give()` returns a non-zero count for the NULL case on purpose. Its
only meaningful caller is `addrenv_drop()`:
```c
if (addrenv_give(addrenv) == 0)
{
... addrenv_destroy(addrenv) ...
}
```
Returning 0 would ask it to destroy an address environment that does not
exist. `addrenv_drop()` happens to check for NULL first, so 0 would be
harmless today, but "there are still users" is the honest answer for a
reference that was never taken.
## Impact
Affected whenever `CONFIG_ARCH_ADDRENV=y` and the calling task has no address
environment:
1. **Every `CONFIG_BUILD_PROTECTED` build with `CONFIG_ARCH_ADDRENV=y`** — no
task ever owns an address environment, so *every* `pthread_create()`
faults. In tree that is `fvp-armv8r:pnsh` and `fvp-armv8r:pnsh_smp`. Both
enable `CONFIG_TESTING_OSTEST`, and ostest reaches `pthread_create()` long
before it finishes, so neither can ever have been run. They require an Arm
Cortex-R82 FVP, which is presumably why this went unnoticed.
Note `CONFIG_ARCH_ADDRENV` is not optional for these configurations: it is
what provides `up_addrenv_kstackalloc()`, required by
`CONFIG_ARCH_KERNEL_STACK`.
2. **`CONFIG_BUILD_KERNEL` builds where a kernel thread calls
`pthread_create()`** — kernel threads never own an address environment
either. In-tree call chain:
```
drivers/video/vnc/vnc_fbdev.c:522 kthread_create("vnc_server", ...,
vnc_server, ...)
drivers/video/vnc/vnc_server.c:334 -> vnc_start_updater(session)
drivers/video/vnc/vnc_updater.c:493 -> pthread_create(...)
```
Not affected: `CONFIG_BUILD_FLAT` (`addrenv_join()` is compiled out), and
`CONFIG_BUILD_KERNEL` user processes (they own a real address environment).
No API, ABI, Kconfig or documentation change. Behaviour is unchanged whenever
the address environment is non-NULL — the patch only adds the NULL case.
## Testing
**Host:** macOS (Darwin 25.5.0), arm64
**Toolchain:** `riscv-none-elf-gcc` 14.2.0 (xPack)
**Emulator:** `qemu-system-riscv64` 10.x
**Configuration:** `rv-virt:knsh64` (`CONFIG_BUILD_KERNEL=y`,
`CONFIG_ARCH_ADDRENV=y`), unmodified in-tree defconfig
```sh
./tools/configure.sh rv-virt:knsh64 && make olddefconfig && make -j10
make export
cd ../apps && ./tools/mkimport.sh -z -x ../nuttx/nuttx-export-*.tar.gz &&
make import
cd ../nuttx
qemu-system-riscv64 -semihosting -M virt -cpu rv64 -smp 1 -nographic -kernel
./nuttx
```
### 1. Reproducing the fault
A kernel thread has no address environment, so calling `pthread_create()`
from
one reaches the same `addrenv_join()` path as a protected build. Added
temporarily to `nx_bringup()`, purely as a test harness — it is *not* part of
this patch:
```c
static pthread_addr_t addrenv_probe_child(pthread_addr_t arg)
{
return NULL;
}
static int addrenv_probe_kthread(int argc, FAR char *argv[])
{
pthread_t tid;
pthread_create(&tid, NULL, addrenv_probe_child, NULL);
return 0;
}
/* in nx_bringup(), after nx_workqueues() */
kthread_create("addrenv_probe", 250, 2048, addrenv_probe_kthread, NULL);
```
**Before this patch:**
```
riscv_exception: EXCEPTION: Store/AMO access fault.
MCAUSE: 0000000000000007, EPC: 0000000080200e90, MTVAL: 0000000000000060
riscv_fault_handler: PANIC!!! Exception = 0000000000000007
dump_assert_info: Current Version: NuttX 13.0.0 a493a9b52f-dirty Jul 26
2026 18:39:10 risc-v
dump_assert_info: Assertion failed panic: at file:
common/riscv_exception.c:142
task: addrenv_probe process: Kernel
```
`EPC` is inside `addrenv_take()`:
```
0000000080200e8a <addrenv_take>:
80200e8a: li a5,1
80200e8c: addi a4,a0,96
80200e90: amoadd.w.aqrl zero,a5,(a4) <-- faults
```
`a0` is the NULL `addrenv`, `+96` is `offsetof(struct addrenv_s, refs)`
(confirmed in DWARF: `refs` has `DW_AT_data_member_location: 96`), and 96 is
`0x60` — exactly the reported `MTVAL`.
**After this patch:** `addrenv_take()` returns without faulting.
That probe then hits a second, unrelated problem — a kernel thread has no
user
heap mapped, so `pthread_create()` goes on to fault in `umm_try_initialize()`
at `0xC0100008`. It is a reproducer for the NULL dereference, not a claim
that
`pthread_create()` from a kernel thread is otherwise supported. In a
protected
build, where the single address space *is* mapped, this patch is all that is
needed.
### 2. No regression — `ostest` on `rv-virt:knsh64`
Full `ostest` run, without the probe, with this patch applied:
```
nsh> ostest
...
user_main: Started with argc=5
...
ostest_main: Exiting with status 0
```
794 lines of output, no assertions or exceptions. `ostest` also passes
without
this patch on the same configuration, as expected — a `CONFIG_BUILD_KERNEL`
user process owns a real address environment and never takes the NULL path.
(Note for anyone reproducing: `apps/testing/ostest/Makefile` sets
`PRIORITY = SCHED_PRIORITY_DEFAULT`, which `Application.mk` encodes as
`nx_priority = 0`; the task is then created at priority 0 and trips
`DEBUGASSERT(sched_priority >= SCHED_PRIORITY_MIN)` in
`nxsched_add_prioritized()`. Setting an explicit `PRIORITY = 100` avoids it.
That is a separate issue, unrelated to this patch.)
### 3. Style
`tools/checkpatch.sh -g <commit>` — `All checks pass.`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
