From 8fb9727ca2c1267e6ae318d0d4dacf5f17cdd5c1 Mon Sep 17 00:00:00 2001 From: Marco Casaroli Date: Tue, 25 Aug 2026 11:07:12 +0200 Subject: [PATCH 1/2] libc, sched: Resolve FDPIC descriptors at module callback entry points. 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) Signed-off-by: Marco Casaroli --- include/nuttx/fdpic.h | 15 ++++++------- include/nuttx/signal.h | 10 +++++++++ libs/libc/dirent/lib_scandir.c | 13 +++++++++++ libs/libc/pthread/pthread_create.c | 14 ++++++++++++ libs/libc/pthread/pthread_once.c | 13 +++++++++++ libs/libc/signal/sig_signal.c | 4 ++++ libs/libc/stdlib/lib_bsearch.c | 11 ++++++++++ libs/libc/stdlib/lib_qsort.c | 35 +++++++++++++++++++++++++++--- sched/mqueue/mq_notify.c | 27 +++++++++++++++++++++++ sched/signal/sig_action.c | 17 +++++++++++++++ sched/signal/sig_notification.c | 24 ++++++++++++++++++++ sched/task/task_create.c | 18 +++++++++++++-- sched/task/task_spawn.c | 12 ++++++++++ sched/timer/timer_create.c | 28 ++++++++++++++++++++++++ 14 files changed, 228 insertions(+), 13 deletions(-) diff --git a/include/nuttx/fdpic.h b/include/nuttx/fdpic.h index 27b997f7f9073..3fc545421c7d0 100644 --- a/include/nuttx/fdpic.h +++ b/include/nuttx/fdpic.h @@ -105,24 +105,23 @@ static inline FAR void *fdpic_callback(FAR void *fn) * carries no module base. Elsewhere fdpic_callback() is enough. * * Input Parameters: - * arg - The one word argument. - * entry - The code address to enter, already resolved from the descriptor. - * got - The module data base to install. + * arg - The one word argument. + * desc - The entry point to enter and the data base to install. * ****************************************************************************/ -static inline void fdpic_invoke(uintptr_t arg, uintptr_t entry, - uintptr_t got) +static inline void fdpic_invoke(uintptr_t arg, + FAR const struct fdpic_desc_s *desc) { - up_fdpic_invoke(arg, entry, got); + up_fdpic_invoke(arg, desc->entry, desc->got); } #else # define fdpic_base() (0) # define fdpic_callback(fn) (fn) -# define fdpic_invoke(arg, entry, got) \ - ((void)(got), (((CODE void (*)(uintptr_t))(uintptr_t)(entry))(arg))) +# define fdpic_invoke(arg, desc) \ + (((CODE void (*)(uintptr_t))(uintptr_t)(desc)->entry)(arg)) #endif /* CONFIG_FDPIC */ diff --git a/include/nuttx/signal.h b/include/nuttx/signal.h index 79fa22b39d501..625177c6677d1 100644 --- a/include/nuttx/signal.h +++ b/include/nuttx/signal.h @@ -35,6 +35,9 @@ #include #include +#ifdef CONFIG_FDPIC +# include +#endif /**************************************************************************** * Pre-processor Definitions @@ -67,7 +70,14 @@ struct sigwork_s { struct work_s work; /* Work queue structure */ union sigval value; /* Data passed with notification */ +#ifdef CONFIG_FDPIC + struct fdpic_desc_s desc; /* Notification function, and the data base + * of a module callback or zero. The base is + * captured at registration and installed + * around the call on the worker thread. */ +#else sigev_notify_function_t func; /* Notification function */ +#endif }; #ifdef __cplusplus diff --git a/libs/libc/dirent/lib_scandir.c b/libs/libc/dirent/lib_scandir.c index c734ff1d243cb..5d620abbf89a9 100644 --- a/libs/libc/dirent/lib_scandir.c +++ b/libs/libc/dirent/lib_scandir.c @@ -31,6 +31,10 @@ #include #include +#ifdef CONFIG_FDPIC +# include +#endif + #include "libc.h" /* The scandir() function is not appropriate for use within the kernel in its @@ -91,6 +95,15 @@ int scandir(FAR const char *path, FAR struct dirent ***namelist, * the original errno value to be able to restore it in case of success. */ +#ifdef CONFIG_FDPIC + /* Resolve the filter descriptor. compar is not resolved here: qsort() + * does it, and resolving twice reads a code address as a descriptor. + */ + + filter = (CODE int (*)(FAR const struct dirent *)) + fdpic_callback((FAR void *)filter); +#endif + errsv = get_errno(); dirp = opendir(path); diff --git a/libs/libc/pthread/pthread_create.c b/libs/libc/pthread/pthread_create.c index 6c87140361cd4..6e9e8e82542d0 100644 --- a/libs/libc/pthread/pthread_create.c +++ b/libs/libc/pthread/pthread_create.c @@ -30,6 +30,10 @@ #include +#ifdef CONFIG_FDPIC +# include +#endif + /**************************************************************************** * Private Functions ****************************************************************************/ @@ -88,6 +92,16 @@ static void pthread_startup(pthread_startroutine_t entry, int pthread_create(FAR pthread_t *thread, FAR const pthread_attr_t *attr, pthread_startroutine_t pthread_entry, pthread_addr_t arg) { +#ifdef CONFIG_FDPIC + /* Resolve the descriptor once, in the public entry point. The new + * thread inherits the creator's D-Space, so it needs only the code + * address. + */ + + pthread_entry = (pthread_startroutine_t) + fdpic_callback((FAR void *)pthread_entry); +#endif + return nx_pthread_create(pthread_startup, thread, attr, pthread_entry, arg); } diff --git a/libs/libc/pthread/pthread_once.c b/libs/libc/pthread/pthread_once.c index ccd854b878843..7b3d9eb3ac3f1 100644 --- a/libs/libc/pthread/pthread_once.c +++ b/libs/libc/pthread/pthread_once.c @@ -33,6 +33,10 @@ #include #include +#ifdef CONFIG_FDPIC +# include +#endif + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -73,6 +77,15 @@ int pthread_once(FAR pthread_once_t *once_control, return EINVAL; } +#ifdef CONFIG_FDPIC + /* Resolve the descriptor here. The value is a local copy, so a later + * call through the same once_control resolves afresh. + */ + + init_routine = (CODE void (*)(void)) + fdpic_callback((FAR void *)init_routine); +#endif + if (!once_control->done) { pthread_mutex_lock(&once_control->mutex); diff --git a/libs/libc/signal/sig_signal.c b/libs/libc/signal/sig_signal.c index 8ed40cf2ef2d6..87794399e7711 100644 --- a/libs/libc/signal/sig_signal.c +++ b/libs/libc/signal/sig_signal.c @@ -71,6 +71,10 @@ _sa_handler_t signal(int signo, _sa_handler_t func) DEBUGASSERT(func != SIG_ERR && func != SIG_HOLD); + /* Not resolved here. nxsig_action() resolves the handler, which covers + * a module calling sigaction() directly as well. + */ + /* Initialize the sigaction structure */ act.sa_handler = func; diff --git a/libs/libc/stdlib/lib_bsearch.c b/libs/libc/stdlib/lib_bsearch.c index a4e3047bf9030..c449480ad9152 100644 --- a/libs/libc/stdlib/lib_bsearch.c +++ b/libs/libc/stdlib/lib_bsearch.c @@ -37,6 +37,10 @@ ****************************************************************************/ #include + +#ifdef CONFIG_FDPIC +# include +#endif #include /**************************************************************************** @@ -114,6 +118,13 @@ FAR void *bsearch(FAR const void *key, FAR const void *base, size_t nel, DEBUGASSERT(base != NULL || nel == 0); DEBUGASSERT(compar != NULL); +#ifdef CONFIG_FDPIC + /* See qsort(): an FDPIC caller passes a descriptor, not a code address */ + + compar = (CODE int (*)(FAR const void *, FAR const void *)) + fdpic_callback((FAR void *)compar); +#endif + for (lim = nel, lower = (const char *)base; lim != 0; lim >>= 1) { middle = lower + (lim >> 1) * width; diff --git a/libs/libc/stdlib/lib_qsort.c b/libs/libc/stdlib/lib_qsort.c index 5646452388f1b..61742d17ef415 100644 --- a/libs/libc/stdlib/lib_qsort.c +++ b/libs/libc/stdlib/lib_qsort.c @@ -45,6 +45,10 @@ #include #include +#ifdef CONFIG_FDPIC +# include +#endif + /**************************************************************************** * Pre-processor Definitions ****************************************************************************/ @@ -156,8 +160,9 @@ static inline FAR char *med3(FAR char *a, FAR char *b, FAR char *c, * ****************************************************************************/ -void qsort(FAR void *base, size_t nel, size_t width, - CODE int(*compar)(FAR const void *, FAR const void *)) +static void qsort_internal(FAR void *base, size_t nel, size_t width, + CODE int(*compar)(FAR const void *, + FAR const void *)) { FAR char *pa; FAR char *pb; @@ -277,7 +282,7 @@ void qsort(FAR void *base, size_t nel, size_t width, if ((r = pb - pa) > width) { - qsort(base, r / width, width, compar); + qsort_internal(base, r / width, width, compar); } if ((r = pd - pc) > width) @@ -289,3 +294,27 @@ void qsort(FAR void *base, size_t nel, size_t width, goto loop; } } + +/**************************************************************************** + * Name: qsort + * + * Description: + * Public entry point. Resolves the comparison function once, then hands + * an ordinary pointer to the implementation, which recurses. + * + ****************************************************************************/ + +void qsort(FAR void *base, size_t nel, size_t width, + CODE int(*compar)(FAR const void *, FAR const void *)) +{ +#ifdef CONFIG_FDPIC + /* An FDPIC module passes the address of a function descriptor, not a + * code address. + */ + + compar = (CODE int (*)(FAR const void *, FAR const void *)) + fdpic_callback((FAR void *)compar); +#endif + + qsort_internal(base, nel, width, compar); +} diff --git a/sched/mqueue/mq_notify.c b/sched/mqueue/mq_notify.c index ea1f35fc27933..5c47a791f7a0b 100644 --- a/sched/mqueue/mq_notify.c +++ b/sched/mqueue/mq_notify.c @@ -34,6 +34,10 @@ #include #include +#if defined(CONFIG_FDPIC) && defined(CONFIG_SIG_EVTHREAD) +# include +#endif + #include "sched/sched.h" #include "mqueue/mqueue.h" @@ -156,6 +160,29 @@ int mq_notify(mqd_t mqdes, FAR const struct sigevent *notification) sizeof(struct sigevent)); msgq->ntpid = rtcb->pid; + +#if defined(CONFIG_FDPIC) && defined(CONFIG_SIG_EVTHREAD) + /* Resolve the callback here, where this still runs in the + * module's context. It fires later on a worker that carries no + * data base, so the base travels with it. The descriptor holds + * the base of the module the callback belongs to, which is not + * always the caller's. The function shares a union with the + * thread ID, so only a SIGEV_THREAD event has one to resolve. + */ + + msgq->ntwork.desc.got = 0; + + if ((notification->sigev_notify & SIGEV_THREAD) != 0 && + fdpic_base() != 0) + { + FAR struct fdpic_desc_s *desc = + (FAR void *)notification->sigev_notify_function; + + msgq->ntevent.sigev_notify_function = + (sigev_notify_function_t)desc->entry; + msgq->ntwork.desc.got = desc->got; + } +#endif } } diff --git a/sched/signal/sig_action.c b/sched/signal/sig_action.c index ab37db83d262b..bf348d431000e 100644 --- a/sched/signal/sig_action.c +++ b/sched/signal/sig_action.c @@ -38,6 +38,10 @@ #include #include +#ifdef CONFIG_FDPIC +# include +#endif + #include "sched/sched.h" #include "group/group.h" #include "signal/signal.h" @@ -326,6 +330,19 @@ int nxsig_action(int signo, FAR const struct sigaction *act, handler = act->sa_handler; +#ifdef CONFIG_FDPIC + /* Resolve the handler here, the innermost common code, so it happens + * exactly once. SIG_ERR, SIG_IGN, SIG_DFL and SIG_HOLD are small + * integers rather than addresses, so exclude them by hand. + */ + + if (handler != SIG_ERR && handler != SIG_IGN && handler != SIG_DFL && + handler != SIG_HOLD) + { + handler = (_sa_handler_t)fdpic_callback((FAR void *)handler); + } +#endif + #ifdef CONFIG_SIG_DEFAULT /* If the caller is setting the handler to SIG_DFL, then we need to * replace this with the correct, internal default signal action handler. diff --git a/sched/signal/sig_notification.c b/sched/signal/sig_notification.c index 86909faf62bc5..9096f5bb46420 100644 --- a/sched/signal/sig_notification.c +++ b/sched/signal/sig_notification.c @@ -34,6 +34,10 @@ #include +#ifdef CONFIG_FDPIC +# include +#endif + #include "sched/sched.h" #include "signal/signal.h" @@ -70,7 +74,23 @@ static void nxsig_notification_worker(FAR void *arg) /* Perform the callback */ +#ifdef CONFIG_FDPIC + /* The worker does not carry the module's data base. Install the base + * captured at registration around the call. A zero base means the + * callback is not a module's. + */ + + if (work->desc.got != 0) + { + fdpic_invoke((uintptr_t)work->value.sival_ptr, &work->desc); + } + else + { + ((sigev_notify_function_t)work->desc.entry)(work->value); + } +#else work->func(work->value); +#endif } #endif /* CONFIG_SIG_EVTHREAD */ @@ -155,7 +175,11 @@ int nxsig_notification(pid_t pid, FAR struct sigevent *event, /* Initialize the work information */ work->value = event->sigev_value; +#ifdef CONFIG_FDPIC + work->desc.entry = (uintptr_t)event->sigev_notify_function; +#else work->func = event->sigev_notify_function; +#endif /* Then queue the work */ diff --git a/sched/task/task_create.c b/sched/task/task_create.c index 4530a742491a0..7b90822cda9f7 100644 --- a/sched/task/task_create.c +++ b/sched/task/task_create.c @@ -37,6 +37,10 @@ #include #include +#ifdef CONFIG_FDPIC +# include +#endif + #include "sched/sched.h" #include "group/group.h" #include "task/task.h" @@ -202,8 +206,18 @@ int task_create_with_stack(FAR const char *name, int priority, FAR void *stack_addr, int stack_size, main_t entry, FAR char * const argv[]) { - int ret = nxtask_create(name, priority, stack_addr, - stack_size, entry, argv, NULL); + int ret; + +#ifdef CONFIG_FDPIC + /* Resolve here, once: this covers task_create() too, which is a plain + * forwarder. The new task inherits the creator's D-Space. + */ + + entry = (main_t)fdpic_callback((FAR void *)entry); +#endif + + ret = nxtask_create(name, priority, stack_addr, + stack_size, entry, argv, NULL); if (ret < 0) { set_errno(-ret); diff --git a/sched/task/task_spawn.c b/sched/task/task_spawn.c index a29db7f893c0a..d223ff259b569 100644 --- a/sched/task/task_spawn.c +++ b/sched/task/task_spawn.c @@ -38,6 +38,10 @@ #include #include +#ifdef CONFIG_FDPIC +# include +#endif + #include "sched/sched.h" #include "group/group.h" #include "task/spawn.h" @@ -335,6 +339,14 @@ int task_spawn(FAR const char *name, main_t entry, pid_t pid = INVALID_PROCESS_ID; int ret; +#ifdef CONFIG_FDPIC + /* Resolve the descriptor once, in the public entry point. The new task + * inherits the creator's D-Space. + */ + + entry = (main_t)fdpic_callback((FAR void *)entry); +#endif + sinfo("name=%s entry=%p file_actions=%p attr=%p argv=%p\n", name, entry, file_actions, attr, argv); diff --git a/sched/timer/timer_create.c b/sched/timer/timer_create.c index 1811f3e68af00..6d4c75772e499 100644 --- a/sched/timer/timer_create.c +++ b/sched/timer/timer_create.c @@ -37,6 +37,10 @@ #include #include +#if defined(CONFIG_FDPIC) && defined(CONFIG_SIG_EVTHREAD) +# include +#endif + #include "sched/sched.h" #include "timer/timer.h" @@ -196,6 +200,30 @@ int timer_create(clockid_t clockid, FAR struct sigevent *evp, /* Yes, copy the entire struct sigevent content */ memcpy(&ret->pt_event, evp, sizeof(struct sigevent)); + +#if defined(CONFIG_FDPIC) && defined(CONFIG_SIG_EVTHREAD) + /* Resolve the callback here, where this still runs in the + * module's context. It fires later on a worker that carries + * no data base, so the base travels with it. The descriptor + * holds the base of the module the callback belongs to, + * which is not always the caller's. The function shares a + * union with the thread ID, so only a SIGEV_THREAD event has + * one to resolve. + */ + + ret->pt_work.desc.got = 0; + + if ((evp->sigev_notify & SIGEV_THREAD) != 0 && + fdpic_base() != 0) + { + FAR struct fdpic_desc_s *desc = + (FAR void *)evp->sigev_notify_function; + + ret->pt_event.sigev_notify_function = + (sigev_notify_function_t)desc->entry; + ret->pt_work.desc.got = desc->got; + } +#endif } else { From 3426186404b954bc2fbb14fa04171d736282ffd3 Mon Sep 17 00:00:00 2001 From: Marco Casaroli Date: Wed, 2 Sep 2026 11:27:30 +0200 Subject: [PATCH 2/2] libs/libc/dirent: Add a blank line after a declaration. 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) Signed-off-by: Marco Casaroli --- libs/libc/dirent/lib_scandir.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/libc/dirent/lib_scandir.c b/libs/libc/dirent/lib_scandir.c index 5d620abbf89a9..bf81022fb17fa 100644 --- a/libs/libc/dirent/lib_scandir.c +++ b/libs/libc/dirent/lib_scandir.c @@ -204,6 +204,7 @@ int scandir(FAR const char *path, FAR struct dirent ***namelist, if (compar) { typedef int (*compar_fn_t)(FAR const void *, FAR const void *); + qsort(list, cnt, sizeof(*list), (compar_fn_t)compar); }