[7/10] libc, sched: Resolve FDPIC descriptors at module callback entry points - #20130
Open
casaroli wants to merge 2 commits into
Open
[7/10] libc, sched: Resolve FDPIC descriptors at module callback entry points#20130casaroli wants to merge 2 commits into
casaroli wants to merge 2 commits into
Conversation
casaroli
requested review from
Donny9,
GUIDINGLI,
Ouss4,
anchao,
davids5,
gustavonihei,
jerpelea,
masayuki2009,
pkarashchenko,
pussuw,
xiaoxiang781216 and
yf13
as code owners
September 13, 2026 17:00
|
The base firmware and an FDPIC module disagree about what a function pointer is. Firmware is not built FDPIC, so to it a pointer is a code address and it branches there. A module passes the address of a two word descriptor instead, because its code and data are placed independently and a bare code address would leave the callee unable to find its own data. A firmware routine that takes a callback therefore branches into the module's data segment and faults. So the ten entry points that can be handed a callback by a module resolve the descriptor before storing or branching to it: qsort, bsearch, pthread_create, signal, sigaction, task_create and task_create_with_stack, task_spawn, pthread_once, scandir, and mq_notify and timer_create with SIGEV_THREAD. Which one resolves matters as much as that one does. Resolving twice would take an already resolved code address for a descriptor and read two words from the instruction stream, so each pointer is resolved exactly once, at the outermost point that sees it. signal() passes its argument through untouched because sigaction() and then nxsig_action() will resolve it, which covers a module calling sigaction() directly as well. qsort() is split so that the public entry resolves and the recursive implementation does not. scandir() resolves its filter but not its comparison function, which it hands to qsort(). Whether a caller is a module at all is asked of the PIC base register, which up_initial_state() sets only for a task that has a D-Space. A plain kernel task therefore reads zero and is left alone. SIGEV_THREAD is the case the register cannot answer, because the callback runs later on a work queue worker that carries no module's base at all. The base is captured instead when the notification is registered, in the module's own context, and installed around the call. All of it is behind CONFIG_FDPIC, which defaults off. Built for mps3-an547:picostest both ways; with it off the entry points compile to what they were. Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Marco Casaroli <marco.casaroli@gmail.com>
nxstyle wants a blank line between a declaration and the statements that follow it. The line is not new, but it sits within three lines of the FDPIC change in this series, so CI reads it as part of the patch. Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Marco Casaroli <marco.casaroli@gmail.com>
casaroli
force-pushed
the
fdpic-callbacks
branch
from
September 13, 2026 17:10
b45d619 to
75cc4ef
Compare
This was referenced Sep 13, 2026
🔗 Cross-repo PR dependenciesThe read-only Build run reported the following dependent PR(s) and fetched head SHA(s): CI run: https://github.com/apache/nuttx/actions/runs/34770810263 |
| */ | ||
|
|
||
| ret->pt_work.got = | ||
| (fdpic_base() != 0 && |
Contributor
There was a problem hiding this comment.
why not save fdpic_base directly
|
|
||
| if (work->got != 0) | ||
| { | ||
| fdpic_invoke((uintptr_t)work->value.sival_ptr, (uintptr_t)work->func, |
Contributor
There was a problem hiding this comment.
why not call fdpic_callback to wrap the callback with got at register site?like other change?
|
|
||
| msgq->ntwork.got = | ||
| (fdpic_base() != 0 && | ||
| (notification->sigev_notify & SIGEV_THREAD) != 0) ? |
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
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.
depends-on: [/pull/20089]
Summary
[5/10]#19942 and[5.5/10]#19940 are merged;[6/10]#20089 relocates an FDPIC object. This makes the firmware callable from one.The base firmware and an FDPIC module disagree about what a function pointer is. Firmware is not built FDPIC, so to it a pointer is a code address and it branches there. A module passes the address of a two word descriptor instead, because its code and data are placed independently and a bare code address would leave the callee unable to find its own data. A firmware routine handed a module's callback therefore branches into the module's data segment and faults.
So the ten entry points that can be handed a callback resolve the descriptor before storing or branching to it:
qsort,bsearch,pthread_create,signal,sigaction,task_createandtask_create_with_stack,task_spawn,pthread_once,scandir, andmq_notifyandtimer_createwithSIGEV_THREAD.Which one resolves matters as much as that one does. Resolving twice would take an already resolved code address for a descriptor and read two words from the instruction stream, so each pointer is resolved exactly once, at the outermost point that sees it.
signal()passes its argument through untouched becausesigaction()and thennxsig_action()will resolve it, which also covers a module callingsigaction()directly.qsort()is split so the public entry resolves and the recursive implementation does not.scandir()resolves its filter but not its comparison function, which it hands toqsort().Whether the caller is a module at all is asked of the PIC base register, which
up_initial_state()sets only for a task that has a D-Space, so a plain kernel task reads zero and is left alone.SIGEV_THREADis the case the register cannot answer, because the callback runs later on a work queue worker carrying no module's base. The base is captured when the notification is registered, in the module's own context, and installed around the call.Impact
All of it is behind
CONFIG_FDPIC, which defaults off and is only selectable whereARCH_HAVE_ELF_FDPICis set.qsort()gains an internal split andscandir()a blank line; everything else is#ifdef'd. With the option off the entry points are what they were.Testing
mps3-an547:picostestbuilds withCONFIG_FDPICoff and on.tools/checkpatch.sh -c -u -m -gpasses.The run time evidence for the descriptors themselves is in
[10/10], whosepimoroni-pico-2-plus:xipfs-fdpiccarriesapps/testing/fs/xipfs: it exercisesqsortandSIGEV_THREADcallbacks from a loaded module, which is what this patch exists to make work.