Skip to content

arch/arm: carry CONTROL over to the fork child on Cortex-M - #19565

Merged
xiaoxiang781216 merged 1 commit into
apache:masterfrom
casaroli:fix-cortexm-fork-ctrlreturn
Jul 28, 2026
Merged

arch/arm: carry CONTROL over to the fork child on Cortex-M#19565
xiaoxiang781216 merged 1 commit into
apache:masterfrom
casaroli:fix-cortexm-fork-ctrlreturn

Conversation

@casaroli

Copy link
Copy Markdown
Contributor

Note: Please adhere to Contributing Guidelines.

Summary

arm_fork() copies the caller's saved system call state to the child so that
the child can return through SYS_syscall_return the way its parent would.
It copies sysreturn and excreturn, but not ctrlreturn.

On Cortex-M in a protected build, arm_svcall.c treats CONTROL as part of that
saved state: it stores the caller's CONTROL in xcp.syscall[].ctrlreturn on
entry and restores it from there on SYS_syscall_return. All three Cortex-M
profiles do this — armv6-m, armv7-m and armv8-m each define the field in
arch/arm/include/<arch>/irq.h and use it symmetrically.

The child's TCB comes from kmm_zalloc(), so the field is zero, and
CONTROL == 0 is nPRIV clear — privileged. The child therefore returns to
user space privileged while its parent returns unprivileged, and runs out its
life with the MPU restrictions its parent is under silently lifted. That is
precisely the isolation CONFIG_BUILD_PROTECTED exists to provide.

Impact

A fork() child in CONFIG_BUILD_PROTECTED on Cortex-M currently executes
with kernel privilege. This restores it to the parent's.

Nothing faults without the fix, which is why it has survived: CONTROL == 0
also selects MSP, which sounds like it should crash immediately, but NuttX
already runs Cortex-M threads on MSP — the parent's saved value is 0x1, nPRIV
set and SPSEL clear — so the two differ only in the privilege bit and there is
no stack change to trip over. Privileged code then passes every test
unprivileged code passes, so ostest cannot see it either.

CONFIG_BUILD_FLAT is unaffected: without CONFIG_LIB_SYSCALL, nsyscalls is
0 and the whole block is skipped. armv7-a and armv7-r are unaffected too; they
carry cpsr instead, and that is already copied.

Testing

Measured on a Raspberry Pi RP2350 (Cortex-M33) in BUILD_PROTECTED, breaking
at the nxtask_start_fork() call during a fork test:

parent  xcp.syscall[0].ctrlreturn = 0x00000001     (nPRIV set, unprivileged)
child   xcp.syscall[0].ctrlreturn = 0x00000000     (nPRIV clear, PRIVILEGED)

With this change both read 0x00000001.

Build-tested mps2-an521:nsh (armv8-m) on macOS/arm64 with Arm GNU
arm-none-eabi-gcc 14.2.Rel1. tools/checkpatch.sh -c -u -m -g clean.

In a protected build arm_svcall.c treats the caller's CONTROL as part of
the saved system call state: it stores it in xcp.syscall[].ctrlreturn on
entry and restores it from there on SYS_syscall_return.  All three
Cortex-M profiles do this -- armv6-m, armv7-m and armv8-m each define the
field in arch/arm/include/<arch>/irq.h and use it symmetrically.

arm_fork_direct() copied sysreturn and excreturn to the child but not
ctrlreturn.  The child's TCB comes from kmm_zalloc(), so the field was
zero, and CONTROL == 0 is nPRIV clear: the child returned to user space
privileged while its parent returned unprivileged.  The child ran out its
life with the MPU restrictions its parent is under silently lifted, which
is the isolation BUILD_PROTECTED exists to provide.

Nothing faults, and that is why this survived.  CONTROL == 0 also selects
MSP, which sounds like it should crash immediately, but NuttX already
runs Cortex-M threads on MSP -- the parent's saved value is 0x1, nPRIV
set and SPSEL clear -- so the two differ only in the privilege bit and
there is no stack change to trip over.  Privileged code then passes every
test unprivileged code passes, so ostest cannot see it either.

Measured on an RP2350 (Cortex-M33) in BUILD_PROTECTED, breaking at the
nxtask_start_fork() call in arm_fork_direct() during task_fork_test:
parent ctrlreturn 0x00000001, child ctrlreturn 0x00000000.  With this
change both read 0x00000001.

BUILD_FLAT is unaffected: without CONFIG_LIB_SYSCALL, nsyscalls is 0 and
the whole block is skipped.  armv7-a and armv7-r are unaffected too; they
carry cpsr instead, and that is already copied.

Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Marco Casaroli <marco.casaroli@gmail.com>
@casaroli
casaroli force-pushed the fix-cortexm-fork-ctrlreturn branch from 3a46a68 to 8538145 Compare July 28, 2026 07:11
@casaroli
casaroli marked this pull request as ready for review July 28, 2026 07:18
@casaroli
casaroli requested review from jerpelea and yf13 as code owners July 28, 2026 07:18
@github-actions github-actions Bot added Arch: arm Issues related to ARM (32-bit) architecture Size: XS The size of the change in this PR is very small labels Jul 28, 2026

@jerpelea jerpelea left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please replace
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
with
Assisted-by: Claude Opus 5 (1M context) noreply@anthropic.com

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

No memory changes detected for:

@casaroli

Copy link
Copy Markdown
Contributor Author

please replace Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com with Assisted-by: Claude Opus 5 (1M context) noreply@anthropic.com

should we add a check in the CI to block it automatically and not let the CI run?

@casaroli
casaroli force-pushed the fix-cortexm-fork-ctrlreturn branch from 8538145 to 5d13ebd Compare July 28, 2026 07:53
@jerpelea

Copy link
Copy Markdown
Contributor

@casaroli please do

@casaroli
casaroli force-pushed the fix-cortexm-fork-ctrlreturn branch from 5d13ebd to 1149269 Compare July 28, 2026 07:55
@casaroli
casaroli requested a review from jerpelea July 28, 2026 08:00
@xiaoxiang781216
xiaoxiang781216 merged commit e499b9f into apache:master Jul 28, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: arm Issues related to ARM (32-bit) architecture Size: XS The size of the change in this PR is very small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants