This is an automated email from the ASF dual-hosted git repository. acassis pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 43694933ce8bfb1800b293b9517ac4c0197aa6f2 Author: Marco Casaroli <[email protected]> AuthorDate: Tue Aug 25 11:07:12 2026 +0200 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) <[email protected]> Signed-off-by: Marco Casaroli <[email protected]> --- include/nuttx/fdpic.h | 66 +++++++++++++++++++++++++++++++------- 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/stdlib/lib_bsearch.c | 11 +++++++ libs/libc/stdlib/lib_qsort.c | 35 ++++++++++++++++++-- sched/mqueue/mq_notify.c | 23 +++++++++++++ sched/signal/sig_action.c | 17 ++++++++++ sched/signal/sig_notification.c | 17 ++++++++++ sched/task/task_create.c | 18 +++++++++-- sched/task/task_spawn.c | 12 +++++++ sched/timer/timer_create.c | 23 +++++++++++++ 13 files changed, 256 insertions(+), 16 deletions(-) diff --git a/include/nuttx/fdpic.h b/include/nuttx/fdpic.h index 27b997f7f90..26185e8dd21 100644 --- a/include/nuttx/fdpic.h +++ b/include/nuttx/fdpic.h @@ -96,33 +96,77 @@ static inline FAR void *fdpic_callback(FAR void *fn) return fn; } +/**************************************************************************** + * Name: fdpic_init + * + * Description: + * Record a callback for a later call from a thread that carries no module + * base. Runs in the caller's own context, which is the only place the + * answer can be had. + * + * A module's function pointer is the address of a descriptor, and the base + * comes from there: a module can hand over a callback that belongs to + * another one. A firmware pointer is a code address and has no base. + * + * Input Parameters: + * desc - The descriptor to fill. + * fn - The callback, as the caller received it. + * + ****************************************************************************/ + +static inline void fdpic_init(FAR struct fdpic_desc_s *desc, + FAR void *fn) +{ + if (fn != NULL && fdpic_base() != 0) + { + *desc = *(FAR struct fdpic_desc_s *)fn; + } + else + { + desc->entry = (uintptr_t)fn; + desc->got = 0; + } +} + /**************************************************************************** * Name: fdpic_invoke * * Description: - * Call a resolved module entry point with the module data base in the PIC - * base register. For a callback that runs on a shared thread, which - * carries no module base. Elsewhere fdpic_callback() is enough. + * Enter a callback recorded by fdpic_init(), with the data base it + * carries in the PIC base register. For a callback that runs on a shared + * thread, which has no base of its own. Elsewhere fdpic_callback() is + * enough. + * + * A zero base means the callback is not a module's, and it is branched to + * directly. * * 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 recorded callback. * ****************************************************************************/ -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); + if (desc->got != 0) + { + up_fdpic_invoke(arg, desc->entry, desc->got); + } + else + { + ((CODE void (*)(uintptr_t))desc->entry)(arg); + } } #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_init(desc, fn) \ + ((desc)->entry = (uintptr_t)(fn), (desc)->got = 0) +# define fdpic_invoke(arg, desc) \ + (((CODE void (*)(uintptr_t))(desc)->entry)(arg)) #endif /* CONFIG_FDPIC */ diff --git a/include/nuttx/signal.h b/include/nuttx/signal.h index 79fa22b39d5..41d42dba568 100644 --- a/include/nuttx/signal.h +++ b/include/nuttx/signal.h @@ -35,6 +35,9 @@ #include <nuttx/wqueue.h> #include <nuttx/sched.h> +#ifdef CONFIG_FDPIC +# include <nuttx/fdpic.h> +#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 func; /* 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 c734ff1d243..5d620abbf89 100644 --- a/libs/libc/dirent/lib_scandir.c +++ b/libs/libc/dirent/lib_scandir.c @@ -31,6 +31,10 @@ #include <errno.h> #include <stdlib.h> +#ifdef CONFIG_FDPIC +# include <nuttx/fdpic.h> +#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 6c87140361c..6e9e8e82542 100644 --- a/libs/libc/pthread/pthread_create.c +++ b/libs/libc/pthread/pthread_create.c @@ -30,6 +30,10 @@ #include <nuttx/pthread.h> +#ifdef CONFIG_FDPIC +# include <nuttx/fdpic.h> +#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 ccd854b8788..7b3d9eb3ac3 100644 --- a/libs/libc/pthread/pthread_once.c +++ b/libs/libc/pthread/pthread_once.c @@ -33,6 +33,10 @@ #include <nuttx/mutex.h> #include <nuttx/debug.h> +#ifdef CONFIG_FDPIC +# include <nuttx/fdpic.h> +#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/stdlib/lib_bsearch.c b/libs/libc/stdlib/lib_bsearch.c index a4e3047bf90..c449480ad91 100644 --- a/libs/libc/stdlib/lib_bsearch.c +++ b/libs/libc/stdlib/lib_bsearch.c @@ -37,6 +37,10 @@ ****************************************************************************/ #include <stdlib.h> + +#ifdef CONFIG_FDPIC +# include <nuttx/fdpic.h> +#endif #include <assert.h> /**************************************************************************** @@ -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 5646452388f..61742d17ef4 100644 --- a/libs/libc/stdlib/lib_qsort.c +++ b/libs/libc/stdlib/lib_qsort.c @@ -45,6 +45,10 @@ #include <sys/param.h> #include <stdlib.h> +#ifdef CONFIG_FDPIC +# include <nuttx/fdpic.h> +#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 @@ loop: 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 @@ loop: 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 ea1f35fc279..225d25dc23f 100644 --- a/sched/mqueue/mq_notify.c +++ b/sched/mqueue/mq_notify.c @@ -34,6 +34,10 @@ #include <nuttx/irq.h> #include <nuttx/sched.h> +#if defined(CONFIG_FDPIC) && defined(CONFIG_SIG_EVTHREAD) +# include <nuttx/fdpic.h> +#endif + #include "sched/sched.h" #include "mqueue/mqueue.h" @@ -156,6 +160,25 @@ 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) + /* Record 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 function shares a + * union with the thread ID, so only a SIGEV_THREAD event has one + * to record. + */ + + if ((notification->sigev_notify & SIGEV_THREAD) != 0) + { + FAR void *fn = (FAR void *)notification->sigev_notify_function; + + fdpic_init(&msgq->ntwork.func, fn); + + msgq->ntevent.sigev_notify_function = + (sigev_notify_function_t)fdpic_callback(fn); + } +#endif } } diff --git a/sched/signal/sig_action.c b/sched/signal/sig_action.c index ab37db83d26..bf348d43100 100644 --- a/sched/signal/sig_action.c +++ b/sched/signal/sig_action.c @@ -38,6 +38,10 @@ #include <nuttx/signal.h> #include <nuttx/spinlock.h> +#ifdef CONFIG_FDPIC +# include <nuttx/fdpic.h> +#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 86909faf62b..a7948c33974 100644 --- a/sched/signal/sig_notification.c +++ b/sched/signal/sig_notification.c @@ -34,6 +34,10 @@ #include <nuttx/signal.h> +#ifdef CONFIG_FDPIC +# include <nuttx/fdpic.h> +#endif + #include "sched/sched.h" #include "signal/signal.h" @@ -70,7 +74,16 @@ static void nxsig_notification_worker(FAR void *arg) /* Perform the callback */ +#ifdef CONFIG_FDPIC + /* The worker does not carry the module's data base. fdpic_invoke() + * installs the one recorded at registration, or branches straight to a + * callback that is not a module's. + */ + + fdpic_invoke((uintptr_t)work->value.sival_ptr, &work->func); +#else work->func(work->value); +#endif } #endif /* CONFIG_SIG_EVTHREAD */ @@ -155,7 +168,11 @@ int nxsig_notification(pid_t pid, FAR struct sigevent *event, /* Initialize the work information */ work->value = event->sigev_value; +#ifdef CONFIG_FDPIC + work->func.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 4530a742491..7b90822cda9 100644 --- a/sched/task/task_create.c +++ b/sched/task/task_create.c @@ -37,6 +37,10 @@ #include <nuttx/kthread.h> #include <nuttx/fs/fs.h> +#ifdef CONFIG_FDPIC +# include <nuttx/fdpic.h> +#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 a29db7f893c..d223ff259b5 100644 --- a/sched/task/task_spawn.c +++ b/sched/task/task_spawn.c @@ -38,6 +38,10 @@ #include <nuttx/kthread.h> #include <nuttx/spawn.h> +#ifdef CONFIG_FDPIC +# include <nuttx/fdpic.h> +#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 1811f3e68af..c756ed2c66e 100644 --- a/sched/timer/timer_create.c +++ b/sched/timer/timer_create.c @@ -37,6 +37,10 @@ #include <nuttx/kmalloc.h> #include <nuttx/spinlock.h> +#if defined(CONFIG_FDPIC) && defined(CONFIG_SIG_EVTHREAD) +# include <nuttx/fdpic.h> +#endif + #include "sched/sched.h" #include "timer/timer.h" @@ -196,6 +200,25 @@ 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) + /* Record 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 function + * shares a union with the thread ID, so only a SIGEV_THREAD + * event has one to record. + */ + + if ((evp->sigev_notify & SIGEV_THREAD) != 0) + { + FAR void *fn = (FAR void *)evp->sigev_notify_function; + + fdpic_init(&ret->pt_work.func, fn); + + ret->pt_event.sigev_notify_function = + (sigev_notify_function_t)fdpic_callback(fn); + } +#endif } else {
