This is an automated email from the ASF dual-hosted git repository. xiaoxiang781216 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 4351f5cd1a88a17ffcab843e7522eaf63e1647b8 Author: zhangyuan29 <[email protected]> AuthorDate: Wed Nov 11 16:27:51 2026 +0800 fs/signalfd: fix notify-path UAF via task-group poll anchor The dispatch path ran a per-device SA_KERNELHAND callback walking dev->fds[] unprotected; a woken higher-prio poller could free the device mid-walk, causing a use-after-free on monkey stop. Anchor poll waiters on the long-lived task group and notify under enter_critical_section(), so the freed device is never dereferenced from the dispatch path. The waiter array is allocated lazily on first signalfd poll and freed at group teardown, so a group that never uses signalfd carries only a NULL pointer. This retires the SA_KERNELHAND dispatch from the signal path: signalfd was its only user, so no code sets or tests the flag anymore (the public macro stays in signal.h for ABI compatibility). A forked child group no longer inherits the parent's signalfd wakeups; its waiter array starts out NULL, matching the per-process semantics of signalfd on Linux. Signed-off-by: zhangyuan29 <[email protected]> Co-authored-by: Claude <[email protected]> --- fs/vfs/fs_signalfd.c | 184 ++++++++++++++++++++++++++++++-------------- include/nuttx/sched.h | 16 ++++ include/nuttx/signalfd.h | 62 +++++++++++++++ sched/group/group_leave.c | 6 ++ sched/signal/sig_dispatch.c | 54 ++++++------- 5 files changed, 238 insertions(+), 84 deletions(-) diff --git a/fs/vfs/fs_signalfd.c b/fs/vfs/fs_signalfd.c index 6f537a1eea1..f86f0b7062b 100644 --- a/fs/vfs/fs_signalfd.c +++ b/fs/vfs/fs_signalfd.c @@ -34,9 +34,13 @@ #include <nuttx/debug.h> #include <nuttx/fs/fs.h> +#include <nuttx/irq.h> +#include <nuttx/kmalloc.h> #include <nuttx/mutex.h> #include <nuttx/poll.h> +#include <nuttx/sched.h> #include <nuttx/signal.h> +#include <nuttx/signalfd.h> #include <sys/signalfd.h> @@ -58,7 +62,6 @@ struct signalfd_priv_s sigset_t sigmask; /* The set of signals caller wishes */ mutex_t mutex; /* Enforces device exclusive access */ uint8_t crefs; /* References counts on signalfd (max: 255) */ - FAR struct pollfd_s *fds[CONFIG_SIGNAL_FD_NPOLLWAITERS]; }; /**************************************************************************** @@ -105,17 +108,6 @@ static struct inode g_signalfd_inode = * Private Functions ****************************************************************************/ -static void signalfd_action(int signo, FAR siginfo_t *info, - FAR void *ucontext) -{ - FAR struct signalfd_priv_s *dev = info->si_user; - - if (sigismember(&dev->sigmask, signo) > 0) - { - poll_notify(dev->fds, CONFIG_SIGNAL_FD_NPOLLWAITERS, POLLIN); - } -} - static int signalfd_file_open(FAR struct file *filep) { FAR struct signalfd_priv_s *dev = filep->f_priv; @@ -139,7 +131,6 @@ static int signalfd_file_open(FAR struct file *filep) static int signalfd_file_close(FAR struct file *filep) { FAR struct signalfd_priv_s *dev = filep->f_priv; - int signo; nxmutex_lock(&dev->mutex); if (dev->crefs > 1) @@ -149,14 +140,6 @@ static int signalfd_file_close(FAR struct file *filep) return OK; } - for (signo = MIN_SIGNO; signo <= MAX_SIGNO; signo++) - { - if (nxsig_ismember(&dev->sigmask, signo) == 1) - { - signal(signo, SIG_DFL); - } - } - nxmutex_unlock(&dev->mutex); nxmutex_destroy(&dev->mutex); fs_heap_free(dev); @@ -229,38 +212,82 @@ static int signalfd_file_poll(FAR struct file *filep, FAR struct pollfd_s *fds, bool setup) { FAR struct signalfd_priv_s *dev = filep->f_priv; + FAR struct task_group_s *group = nxsched_self()->group; + FAR struct pollfd_s **cand = NULL; + irqstate_t flags; sigset_t mask; int ret = 0; int i; - nxmutex_lock(&dev->mutex); - if (!setup) + /* The waiter array is allocated lazily so a group that never polls a + * signalfd carries only a NULL pointer. Allocate the candidate outside + * the critical section (allocation must not run under + * enter_critical_section); it is installed under the lock and freed here + * if another concurrent poll setup won the race. tg_sigfds only ever + * transitions NULL->set (freed at group teardown), so the unlocked + * pre-check is a safe hint. + */ + + if (setup && group->tg_sigfds == NULL) { - /* This is a request to tear down the poll. */ + cand = kmm_zalloc(CONFIG_SIGNAL_FD_NPOLLWAITERS * sizeof(*cand)); + if (cand == NULL) + { + return -ENOMEM; + } + } - FAR struct pollfd_s **slot = (FAR struct pollfd_s **)fds->priv; + /* The poll waiters are anchored on the task group and are notified from + * the signal dispatch path, which may run in interrupt context. Use + * enter_critical_section() (switch-aware, so poll_notify()'s nxsem_post() + * may switch out safely) rather than the device mutex, which the dispatch + * path could not take. + */ - /* Remove all memory of the poll setup */ + flags = enter_critical_section(); + if (!setup) + { + /* This is a request to tear down the poll. Locate our slot in the + * group anchor by pointer and clear it. + */ + + if (group->tg_sigfds != NULL) + { + for (i = 0; i < CONFIG_SIGNAL_FD_NPOLLWAITERS; i++) + { + if (group->tg_sigfds[i] == fds) + { + group->tg_sigfds[i] = NULL; + break; + } + } + } - *slot = NULL; fds->priv = NULL; goto out; } - /* This is a request to set up the poll. Find an available - * slot for the poll structure reference + /* Install the lazily-allocated array if it is still missing. */ + + if (group->tg_sigfds == NULL) + { + group->tg_sigfds = cand; + cand = NULL; + } + + /* This is a request to set up the poll. Find an available slot in the + * group anchor and bind the poll structure to it. The device pointer is + * stashed in fds->priv so signalfd_notify() can filter by this fd's mask; + * this is safe because an occupied slot implies poll_setup() holds a file + * reference, so the device cannot be closed/freed while bound. */ for (i = 0; i < CONFIG_SIGNAL_FD_NPOLLWAITERS; i++) { - /* Find an available slot */ - - if (!dev->fds[i]) + if (!group->tg_sigfds[i]) { - /* Bind the poll structure and this slot */ - - dev->fds[i] = fds; - fds->priv = &dev->fds[i]; + group->tg_sigfds[i] = fds; + fds->priv = dev; break; } } @@ -271,7 +298,7 @@ static int signalfd_file_poll(FAR struct file *filep, goto out; } - /* Notify the POLLIN event if the counter is not zero */ + /* Notify the POLLIN event if a subscribed signal is already pending */ mask = nxsig_pendingset(NULL); sigandset(&mask, &mask, &dev->sigmask); @@ -281,10 +308,67 @@ static int signalfd_file_poll(FAR struct file *filep, } out: - nxmutex_unlock(&dev->mutex); + leave_critical_section(flags); + + /* Free the candidate array if another poll setup installed one first. */ + + if (cand != NULL) + { + kmm_free(cand); + } + return ret; } +/**************************************************************************** + * Name: signalfd_notify + * + * Description: + * Notify the signalfd poll waiters of a task group that signo has been + * delivered. Called from the signal dispatch path, which may run in + * interrupt context; enter_critical_section() serializes against + * signalfd_file_poll() setup/teardown and is switch-aware, so the + * nxsem_post() inside poll_notify() may switch out safely. + * + * Only the fds whose device mask contains signo are notified, so that + * several signalfd fds sharing one poll loop do not get spurious POLLIN + * for signals they did not subscribe to. An occupied slot implies + * poll_setup() holds a file reference on that fd, so fds->priv (the + * device) is guaranteed live for the duration of this walk. + * + ****************************************************************************/ + +void signalfd_notify(FAR struct task_group_s *group, int signo) +{ + irqstate_t flags; + int i; + + /* A group that never set up a signalfd poll has no waiter array. */ + + if (group->tg_sigfds == NULL) + { + return; + } + + flags = enter_critical_section(); + for (i = 0; i < CONFIG_SIGNAL_FD_NPOLLWAITERS; i++) + { + FAR struct pollfd_s *fds = group->tg_sigfds[i]; + + if (fds != NULL) + { + FAR struct signalfd_priv_s *dev = fds->priv; + + if (sigismember(&dev->sigmask, signo) > 0) + { + poll_notify(&fds, 1, POLLIN); + } + } + } + + leave_critical_section(flags); +} + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -335,9 +419,7 @@ int signalfd(int fd, FAR const sigset_t *mask, int flags) { FAR struct signalfd_priv_s *dev; FAR struct file *filep = NULL; - struct sigaction act; int ret = EINVAL; - int signo; if (flags & ~(SFD_CLOEXEC | SFD_NONBLOCK)) { @@ -380,28 +462,16 @@ int signalfd(int fd, FAR const sigset_t *mask, int flags) } dev = filep->f_priv; - for (signo = MIN_SIGNO; signo <= MAX_SIGNO; signo++) - { - if (nxsig_ismember(&dev->sigmask, signo) == 1) - { - signal(signo, SIG_DFL); - } - } } + /* Update the subscribed signal set. signalfd_notify() reads dev->sigmask + * live when filtering, so a replaced mask takes effect immediately with no + * per-signal action to (un)register. + */ + dev->sigmask = *mask; nxsig_delset(&dev->sigmask, SIGKILL); nxsig_delset(&dev->sigmask, SIGSTOP); - act.sa_sigaction = signalfd_action; - act.sa_flags = SA_KERNELHAND | SA_SIGINFO; - act.sa_user = dev; - for (signo = MIN_SIGNO; signo <= MAX_SIGNO; signo++) - { - if (nxsig_ismember(&dev->sigmask, signo) == 1) - { - nxsig_action(signo, &act, NULL, false); - } - } if (filep != NULL) { diff --git a/include/nuttx/sched.h b/include/nuttx/sched.h index 5d54bb2a439..9db6e350d9c 100644 --- a/include/nuttx/sched.h +++ b/include/nuttx/sched.h @@ -544,6 +544,22 @@ struct task_group_s #ifndef CONFIG_DISABLE_ALL_SIGNALS sq_queue_t tg_sigpendingq; /* List of pending signals */ #endif /* !CONFIG_DISABLE_ALL_SIGNALS */ + +#ifdef CONFIG_SIGNAL_FD + /* signalfd poll waiters. Signal notification is delivered from the signal + * dispatch path (possibly interrupt context), which cannot take the + * per-device sleeping mutex. Anchoring the poll waiters on the long-lived + * task group instead of the fd-scoped device removes the dangling-pointer + * and use-after-free hazards of the old per-device kernel-action callback. + * + * The waiter array is allocated lazily on first signalfd poll setup and + * freed at group teardown, so a group that never uses signalfd carries + * only this NULL pointer rather than a fixed-size array. + */ + + FAR struct pollfd_s **tg_sigfds; +#endif + #ifdef CONFIG_SIG_DEFAULT sigset_t tg_sigdefault; /* Set of signals set to the default action */ #endif diff --git a/include/nuttx/signalfd.h b/include/nuttx/signalfd.h new file mode 100644 index 00000000000..21a8a09c211 --- /dev/null +++ b/include/nuttx/signalfd.h @@ -0,0 +1,62 @@ +/**************************************************************************** + * include/nuttx/signalfd.h + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ****************************************************************************/ + +#ifndef __INCLUDE_NUTTX_SIGNALFD_H +#define __INCLUDE_NUTTX_SIGNALFD_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +/**************************************************************************** + * Public Function Prototypes + ****************************************************************************/ + +struct task_group_s; + +/**************************************************************************** + * Name: signalfd_notify + * + * Description: + * Notify the signalfd poll waiters of the target task group that a signal + * has been delivered. Called from the signal dispatch path (which may run + * in interrupt context), so this must not take any sleeping lock. + * + * The poll waiters are anchored on the long-lived task group rather than + * the fd-scoped device, so this path never dereferences a device that a + * concurrent close() may have freed. + * + * Input Parameters: + * group - The task group the signal was delivered to. + * signo - The signal number that was delivered. + * + ****************************************************************************/ + +#ifdef CONFIG_SIGNAL_FD +void signalfd_notify(FAR struct task_group_s *group, int signo); +#else +# define signalfd_notify(group, signo) +#endif + +#endif /* __INCLUDE_NUTTX_SIGNALFD_H */ diff --git a/sched/group/group_leave.c b/sched/group/group_leave.c index 9e6dc112301..1364bf4534c 100644 --- a/sched/group/group_leave.c +++ b/sched/group/group_leave.c @@ -290,6 +290,12 @@ void group_drop(FAR struct task_group_s *group) if (group->tg_flags & GROUP_FLAG_DELETED) { +#ifdef CONFIG_SIGNAL_FD + /* Free the lazily-allocated signalfd poll waiter array, if any. */ + + kmm_free(group->tg_sigfds); +#endif + /* Release the group container itself */ kmm_free(group); diff --git a/sched/signal/sig_dispatch.c b/sched/signal/sig_dispatch.c index 14e80f02b7c..409c5e134ad 100644 --- a/sched/signal/sig_dispatch.c +++ b/sched/signal/sig_dispatch.c @@ -40,6 +40,7 @@ #include <nuttx/nuttx.h> #include <nuttx/arch.h> #include <nuttx/signal.h> +#include <nuttx/signalfd.h> #include <nuttx/queue.h> #include "sched/sched.h" @@ -211,24 +212,6 @@ static int nxsig_queue_action(FAR struct tcb_s *stcb, return ret; } - -/**************************************************************************** - * Name: nxsig_dispatch_kernel_action - ****************************************************************************/ - -static void nxsig_dispatch_kernel_action(FAR struct tcb_s *stcb, - FAR siginfo_t *info) -{ - FAR struct task_group_s *group = stcb->group; - FAR sigactq_t *sigact; - - sigact = nxsig_find_action(group, info->si_signo); - if (sigact && (sigact->act.sa_flags & SA_KERNELHAND)) - { - info->si_user = sigact->act.sa_user; - (sigact->act.sa_sigaction)(info->si_signo, info, NULL); - } -} #endif /* CONFIG_ENABLE_ALL_SIGNALS */ /**************************************************************************** @@ -769,6 +752,19 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, FAR siginfo_t *info, { sigpend = nxsig_add_pendingsignal(stcb, info, group_dispatch, source); + + /* The pending entry may be released as soon as the critical + * section is left (see nxsig_unmaskpendingsignal()), so the + * signalfd poll waiters must be notified while it is still + * owned. The waiters are anchored on the task group, so this + * never touches an fd-scoped device a concurrent close may + * free. + */ + + if (sigpend != NULL) + { + signalfd_notify(stcb->group, sigpend->info.si_signo); + } } #endif } @@ -781,6 +777,19 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, FAR siginfo_t *info, { sigpend = nxsig_add_pendingsignal(stcb, info, group_dispatch, source); + + /* The pending entry may be released as soon as the critical + * section is left (see nxsig_unmaskpendingsignal()), so the + * signalfd poll waiters must be notified while it is still + * owned. The waiters are anchored on the task group, so this + * never touches an fd-scoped device a concurrent close may + * free. + */ + + if (sigpend != NULL) + { + signalfd_notify(stcb->group, sigpend->info.si_signo); + } } } @@ -930,15 +939,6 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, FAR siginfo_t *info, } #endif - /* Dispatch kernel action, if needed, in case a pending signal was added */ - - if (sigpend != NULL) - { -#ifdef CONFIG_ENABLE_ALL_SIGNALS - nxsig_dispatch_kernel_action(stcb, &sigpend->info); -#endif - } - /* In case nxsig_ismember failed due to an invalid signal number */ if (masked < 0)
