Conversation
royzah
requested review from
Donny9,
GUIDINGLI,
anchao,
hartmannathan,
jerpelea,
linguini1,
lupyuen,
masayuki2009,
pkarashchenko,
pussuw,
xiaoxiang781216 and
yf13
as code owners
September 30, 2026 17:22
linguini1
requested changes
Sep 30, 2026
linguini1
left a comment
Contributor
There was a problem hiding this comment.
Please split into smaller patches and include test logs.
acassis
previously approved these changes
Sep 30, 2026
Contributor
|
Strange, membrowse didn't show the memory change for this PR |
|
Contributor
Author
The memory manager block was indented one level too deep. No code changes. Signed-off-by: Royyan Zahir <royzah@gmail.com>
Contributor
Author
|
Reworked as asked: on master by itself, no BUILD_KERNEL guard, the checks moved into the functions, plus a docs page and an ostest case. Fresh logs in the body. |
Contributor
Author
|
Pushed the rework from the review on #20425: checks moved into the internal functions, the RAWIO flag and register_rawdriver() are gone, BCH checks in bch_open(). Fresh logs in the body. |
Contributor
Author
|
Full log of the flat run,
|
| FAR struct bchlib_s *bch; | ||
| int ret = OK; | ||
|
|
||
| if (!nxsched_capable(PR_CAP_RAWIO)) |
Contributor
There was a problem hiding this comment.
remove, already check by:
4c795c8#diff-45fe6e5a6529a5265d9338cbbfd4318580490a087320d8ab3645542c07dec51fR152
Contributor
Author
There was a problem hiding this comment.
hmm that one only catches BLOCK/MTD. a node from bchdev_register() is a plain char driver, so file_vopen can't tell its raw storage. so bch_open() covers that case
Contributor
There was a problem hiding this comment.
but bch_register is only called inside file_open?
Contributor
Author
There was a problem hiding this comment.
not only tho. block_proxy makes /dev/tmpcXXXXXX (just a counter, easy to guess) and unlinks it only after the open, so another task can grab it in that gap. and apps can call bchdev_register directly too, eg examples/mtdrwb makes /dev/mtd0 and it stays. bch_open is the one spot both go thru. ok to keep it?
A task group holds PR_CAP_RAWIO, PR_CAP_SPAWN and PR_CAP_ADMIN, inherits them from its creator and can only drop them. Every build: the kernel and init start with all three, so nothing changes until a task drops one. CONFIG_SCHED_CAPABILITIES, off with DEFAULT_SMALL, lets a board short of flash leave the checks out. Signed-off-by: Royyan Zahir <royzah@gmail.com>
Indent the bch_seek() switch cases so the file passes nxstyle. Signed-off-by: Royyan Zahir <royzah@gmail.com>
Opening a block or MTD node, mount() and umount2() need it, and so does a BCH character node, which checks in its own open(): a node made by bchdev_register() is a character driver, so the inode type cannot tell it apart. The checks sit in file_vopen(), nx_mount() and nx_umount2(), which every path reaches; kernel threads hold every capability. Signed-off-by: Royyan Zahir <royzah@gmail.com>
exec_internal(), which posix_spawn(), execve() and exec() all reach, nxtask_spawn_create() and nxtask_create() check it. nxthread_create() does not: kernel threads go through it too. Signed-off-by: Royyan Zahir <royzah@gmail.com>
Indentation and comment fixes only, so the next change passes the style check. Signed-off-by: Royyan Zahir <royzah@gmail.com>
boardctl(BOARDIOC_RESET) and boardctl(BOARDIOC_POWEROFF) refuse a task group without it. Signed-off-by: Royyan Zahir <royzah@gmail.com>
What each capability guards, the prctl() interface, inheritance through the task group, and where the checks sit. Signed-off-by: Royyan Zahir <royzah@gmail.com>
Contributor
Author
|
pushed. CI red was nucleo-g431rb:cansock out of flash, it only had ~250 B left. so added CONFIG_SCHED_CAPABILITIES (off with DEFAULT_SMALL) and turned it off there, now it builds 32 B over master. also regrouped commits per cap like u asked. fresh logs in the body |
nucleo-g431rb:cansock and b-g431b-esc1:cansock fill their 128 KB of flash to the last few hundred bytes, and the capability checks overflow them. Signed-off-by: Royyan Zahir <royzah@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note: Please adhere to Contributing Guidelines.
Summary
Any process could open raw storage, spawn programs, mount file systems and reset the board, and had no way to give that up.
nx_start.cpasses the check; whitespace onlyschedPR_CAP_RAWIO,PR_CAP_SPAWN,PR_CAP_ADMIN, inherited from its creator;prctl(PR_CAPS_DROP)only removes,PR_CAPS_GETreads;CONFIG_SCHED_CAPABILITIES, off withDEFAULT_SMALLbchdev_driver.cpasses the check; whitespace onlyfsRAWIO: opening a block or MTD node, a BCH node in its ownopen(),nx_mount(),nx_umount2()sched, binfmtSPAWN:exec_internal(),nxtask_spawn_create(),nxtask_create()boardctl.cpasses the check; whitespace onlyboardsADMIN:boardctlreset and power-offDocumentationimplementation/capabilities.rstboards/stm32g4nucleo-g431rb:cansockandb-g431b-esc1:cansockleave the option out: each had a few hundred bytes of its 128 KB flash leftThe checks sit in the internal functions every path reaches, not in the syscall layer, so they hold in flat, protected and kernel builds, and kernel code acting for a process is held to that process's set. Kernel threads hold all three.
Impact
Every build with
CONFIG_SCHED_CAPABILITIES, on unlessDEFAULT_SMALL. Nothing changes until a process drops a capability: the kernel andinitstart with all three. Off, every check compiles away: with the cross-process checks too,nucleo-g431rb:cansockbuilds at 129532 B of text andb-g431b-esc1:cansockat 129524 B.Testing
Host: Ubuntu 24.04, x86_64. Builds and QEMU runs in
ghcr.io/apache/nuttx/apache-nuttx-ci-linux(QEMU 6.2, Arm GNU GCC 13.2, xPack RISC-V GCC 14.3). Hardware: i.MX93 (Cortex-A55), PX4 kernel build.qemu-armv8a:nsh, flat, with the newostestcasecaps_test: PASSED;ostestpassesqemu-armv8a:knshostestpasses;hellorv-virt:knsh64hello;osteststops afterStarted user_main at PID=6, as master doesPR_CAPS_GETreads 0The
ostestcase drops each capability in a child task and checksEPERM; it goes to nuttx-apps once this lands.qemu-armv8a:nsh, flat, with the newostestcase: the caps part and the endqemu-armv8a:knsh:hello, thenostestrv-virt:knsh64:hello, thenostest; identical to master'si.MX93, PX4 kernel build, an earlier revision of this series:
tests isolationtools/checkpatch.sh -c -u -m -gclean.