From 4b130fe6ef95753b7280edd34b9e99d02c6b3ed6 Mon Sep 17 00:00:00 2001 From: Adil Alperen Ciftci Date: Wed, 4 Nov 2026 02:33:58 +0300 Subject: [PATCH 1/5] dfs/posix: enforce bounds check and clean unregister in signalfd Validate array bounds on signalfd notification registration and remove the unsafe global index counter. Tie the registered process lifetime to the registration: each slot in sfd->lwp[] holds an LWP reference, and the listener node carries a detach hook that the process exit path runs under LWP_LOCK to unlink the node, clear the slot and drop the reference. Final close therefore never walks a freed process, and a child that registered through an inherited descriptor is unregistered when it exits. Move every change of lwp->signalfd_notify_head behind lwp_signal_notify_* helpers in the lwp core, which take LWP_LOCK, the lock lwp_signal_notify() traverses the list under. The lock order is LWP_LOCK -> sfd->lock, the same order the signal path already uses; sfd->lock is never held while taking LWP_LOCK, and a node is only unlinked under LWP_LOCK, so no in-flight notification can be inside it when it is freed. Tear down the vnode, context and mutex before fd_release() when the initial registration fails, and return instead of falling through to freed objects. Propagate registration failures: read returns the error and poll reports POLLERR before anything is added to the poll queue. Fixes: #11933 Signed-off-by: Adil Alperen Ciftci --- components/libc/posix/io/signalfd/signalfd.c | 270 +++++++++++++++---- components/lwp/lwp.h | 9 + components/lwp/lwp_pid.c | 1 + components/lwp/lwp_signal.c | 77 ++++++ components/lwp/lwp_signal.h | 43 +++ 5 files changed, 344 insertions(+), 56 deletions(-) diff --git a/components/libc/posix/io/signalfd/signalfd.c b/components/libc/posix/io/signalfd/signalfd.c index b4a08fa94675..496a3096f031 100644 --- a/components/libc/posix/io/signalfd/signalfd.c +++ b/components/libc/posix/io/signalfd/signalfd.c @@ -20,10 +20,8 @@ #include #define SIGNALFD_MUTEX_NAME "signalfd" -#define SIGINFO_MAX 10 -#define SIGNALFD_SHART_MAX RT_SIGNALFD_MAX_NUM - -static int is_head_init = 0; +#define SIGINFO_MAX 10 +#define SIGNALFD_SHART_MAX RT_SIGNALFD_MAX_NUM struct rt_signalfd_ctx { @@ -44,11 +42,24 @@ static ssize_t signalfd_read(struct dfs_file *file, void *buf, size_t count, off #endif static int signalfd_add_notify(struct rt_signalfd_ctx *sfd); -static const struct dfs_file_ops signalfd_fops = -{ - .close = signalfd_close, - .poll = signalfd_poll, - .read = signalfd_read, +/* + * Lock order: LWP_LOCK(lwp) -> sfd->lock. + * + * lwp_signal_notify() walks lwp->signalfd_notify_head under LWP_LOCK and the + * callback takes sfd->lock inside it. The listener list is therefore only + * changed through the lwp_signal_notify_* helpers, which take LWP_LOCK, and + * sfd->lock is never held while calling them. A node is unlinked under + * LWP_LOCK, so no notification can still be inside it when it is freed. + * + * Each registered process holds one LWP reference. The slot in sfd->lwp[] is + * the registration: whoever clears a slot (close or the process exit hook) + * drops that reference, so the pointer stays valid until it is cleared. + */ + +static const struct dfs_file_ops signalfd_fops = { + .close = signalfd_close, + .poll = signalfd_poll, + .read = signalfd_read, }; /** @@ -60,14 +71,38 @@ static int signalfd_close(struct dfs_file *file) { struct rt_signalfd_ctx *sfd; + if (!file || !file->vnode) + { + return 0; + } + if (file->vnode->ref_count != 1) + { return 0; + } sfd = file->vnode->data; if (sfd) { + for (int i = 0; i < SIGNALFD_SHART_MAX; i++) + { + struct rt_lwp *lwp; + + rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); + lwp = sfd->lwp[i]; + sfd->lwp[i] = RT_NULL; + rt_mutex_release(&sfd->lock); + + if (lwp) + { + /* the slot held a reference, so lwp is still valid here */ + lwp_signal_notify_remove_queue(lwp, &sfd->signalfd_queue); + lwp_ref_dec(lwp); + } + } rt_mutex_detach(&sfd->lock); + file->vnode->data = RT_NULL; rt_free(sfd); } @@ -89,13 +124,19 @@ static int signalfd_poll(struct dfs_file *file, struct rt_pollreq *req) { sfd = file->vnode->data; + if (signalfd_add_notify(sfd) < 0) + { + return POLLERR; + } + rt_poll_add(&sfd->signalfd_queue, req); - signalfd_add_notify(sfd); rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); if (sfd->sig_num) + { events |= POLLIN; + } rt_mutex_release(&sfd->lock); } @@ -132,10 +173,14 @@ static ssize_t signalfd_read(struct dfs_file *file, void *buf, size_t count, off rt_err_t ret = -1; if (sizeof(struct signalfd_siginfo) > count) + { return -1; + } if (buf == RT_NULL) + { return -1; + } buffer = (struct signalfd_siginfo *)buf; user_buf_num = count / sizeof(struct signalfd_siginfo); @@ -144,7 +189,12 @@ static ssize_t signalfd_read(struct dfs_file *file, void *buf, size_t count, off { sfd = file->vnode->data; - signalfd_add_notify(sfd); + ret = signalfd_add_notify(sfd); + if (ret < 0) + { + return ret; + } + if ((sfd->sig_num == 0) && (file->flags & O_NONBLOCK)) { ret = -EAGAIN; @@ -171,9 +221,9 @@ static ssize_t signalfd_read(struct dfs_file *file, void *buf, size_t count, off } } - for (int j = 0; j < sfd->sig_num; j ++) + for (int j = 0; j < sfd->sig_num; j++) { - memcpy(&sfd->info[j], &sfd->info[i ++], sizeof(struct signalfd_siginfo)); + memcpy(&sfd->info[j], &sfd->info[i++], sizeof(struct signalfd_siginfo)); } rt_mutex_release(&sfd->lock); @@ -207,9 +257,69 @@ static void signalfd_callback(rt_wqueue_t *signalfd_queue, int signum) sfd->sig_num += 1; } rt_mutex_release(&sfd->lock); - rt_wqueue_wakeup(signalfd_queue, (void*)POLLIN); + rt_wqueue_wakeup(signalfd_queue, (void *)POLLIN); + } + } +} + +/** + * @brief Exit hook for a process registered on a signalfd. + * @param signalfd_queue Pointer to the signalfd queue. + * @param lwp The exiting process. + * @note Runs under LWP_LOCK from lwp_signal_notify_detach() after the + * listener node has been unlinked. Clears the process slot and drops + * the reference the registration took, unless close already did. + */ +static void signalfd_detach(rt_wqueue_t *signalfd_queue, struct rt_lwp *lwp) +{ + struct rt_signalfd_ctx *sfd; + int drop_ref = 0; + + sfd = rt_container_of(signalfd_queue, struct rt_signalfd_ctx, signalfd_queue); + + rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); + for (int i = 0; i < SIGNALFD_SHART_MAX; i++) + { + if (sfd->lwp[i] == lwp) + { + sfd->lwp[i] = RT_NULL; + drop_ref = 1; + break; + } + } + rt_mutex_release(&sfd->lock); + + if (drop_ref) + { + lwp_ref_dec(lwp); + } +} + +/** + * @brief Looks up the slot of a process, or a free slot, in a signalfd context. + * @param sfd Pointer to the signalfd context. + * @param lwp Process to look for. + * @param empty_slot Receives the first free slot index, or -1. + * @return 1 if the process is already registered, 0 otherwise. + * @note Caller holds sfd->lock. + */ +static int signalfd_find_slot(struct rt_signalfd_ctx *sfd, struct rt_lwp *lwp, int *empty_slot) +{ + *empty_slot = -1; + + for (int i = 0; i < SIGNALFD_SHART_MAX; i++) + { + if (sfd->lwp[i] == lwp) + { + return 1; + } + if (sfd->lwp[i] == RT_NULL && *empty_slot == -1) + { + *empty_slot = i; } } + + return 0; } /** @@ -220,56 +330,98 @@ static void signalfd_callback(rt_wqueue_t *signalfd_queue, int signum) static int signalfd_add_notify(struct rt_signalfd_ctx *sfd) { struct rt_lwp_notify *lwp_notify; - rt_err_t ret = -1; - rt_slist_t *node; - int is_lwp = 0; + rt_err_t ret = 0; + int registered; + int empty_slot; + struct rt_lwp *curr_lwp; + + if (!sfd) + { + return -EINVAL; + } + + curr_lwp = lwp_self(); + if (!curr_lwp) + { + return -EINVAL; + } rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); + registered = signalfd_find_slot(sfd, curr_lwp, &empty_slot); + rt_mutex_release(&sfd->lock); - for (int i = 0; i < is_head_init; i++) + if (registered) { - if (sfd->lwp[i]) - { - if (sfd->lwp[i] == lwp_self()) - { - is_lwp = 1; - } - } + return 0; } - if (is_lwp == 0) + if (empty_slot < 0) { - sfd->lwp[is_head_init] = lwp_self(); + rt_set_errno(EMFILE); + return -EMFILE; + } - if (is_head_init == 0) - { - rt_slist_init(&sfd->lwp[is_head_init]->signalfd_notify_head); - } + lwp_notify = (struct rt_lwp_notify *)rt_malloc(sizeof(struct rt_lwp_notify)); + if (!lwp_notify) + { + rt_set_errno(ENOMEM); + return -ENOMEM; + } + lwp_notify->notify = signalfd_callback; + lwp_notify->detach = signalfd_detach; + lwp_notify->signalfd_queue = &sfd->signalfd_queue; - lwp_notify = (struct rt_lwp_notify *)rt_malloc(sizeof(struct rt_lwp_notify)); - if (lwp_notify) - { - lwp_notify->notify = signalfd_callback; - lwp_notify->signalfd_queue = &sfd->signalfd_queue; - rt_slist_append(&sfd->lwp[is_head_init]->signalfd_notify_head, &(lwp_notify->list_node)); + /* the slot owns this reference until close or process exit clears it */ + lwp_ref_inc(curr_lwp); - is_head_init ++; - ret = 0; - } - else + rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); + if (sfd->lwp[empty_slot] != RT_NULL) + { + /* lost the slot meanwhile: look again */ + registered = signalfd_find_slot(sfd, curr_lwp, &empty_slot); + } + if (!registered && empty_slot >= 0) + { + sfd->lwp[empty_slot] = curr_lwp; + } + rt_mutex_release(&sfd->lock); + + if (registered || empty_slot < 0) + { + lwp_ref_dec(curr_lwp); + rt_free(lwp_notify); + if (registered) { - rt_slist_for_each(node, &sfd->lwp[is_head_init]->signalfd_notify_head) - { - struct rt_lwp_notify *n = rt_slist_entry(node, struct rt_lwp_notify, list_node); - rt_slist_remove(&sfd->lwp[is_head_init]->signalfd_notify_head, &n->list_node); - rt_free(n); - } - rt_set_errno(ENOMEM); + return 0; } + rt_set_errno(EMFILE); + return -EMFILE; } + lwp_signal_notify_add(curr_lwp, lwp_notify); + + /* + * close() clears the slot first and unlinks listeners afterwards. If the + * slot is gone now, close is running and may already have walked the + * list; take the node back ourselves if it is still there. The reference + * was dropped by whoever cleared the slot. + */ + rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); + if (sfd->lwp[empty_slot] != curr_lwp) + { + ret = -EBADF; + } rt_mutex_release(&sfd->lock); + if (ret < 0) + { + if (lwp_signal_notify_remove(curr_lwp, lwp_notify)) + { + rt_free(lwp_notify); + } + rt_set_errno(EBADF); + } + return ret; } @@ -290,7 +442,9 @@ static int signalfd_do(int fd, const sigset_t *mask, int flags) { fd = fd_new(); if (fd < 0) + { return -1; + } ret = fd; df = fd_get(fd); @@ -306,7 +460,7 @@ static int signalfd_do(int fd, const sigset_t *mask, int flags) dfs_vnode_init(df->vnode, FT_REGULAR, &signalfd_fops); df->vnode->data = sfd; - for (int i = 0; i < is_head_init; i++) + for (int i = 0; i < SIGNALFD_SHART_MAX; i++) { sfd->lwp[i] = RT_NULL; } @@ -314,24 +468,28 @@ static int signalfd_do(int fd, const sigset_t *mask, int flags) sigemptyset(&sfd->sigmask); memcpy(&sfd->sigmask, mask, sizeof(sigset_t)); + sfd->sig_num = 0; + rt_mutex_init(&sfd->lock, SIGNALFD_MUTEX_NAME, RT_IPC_FLAG_FIFO); rt_wqueue_init(&sfd->signalfd_queue); if (signalfd_add_notify(sfd) < 0) { - is_head_init = 0; - fd_release(fd); + /* nothing is registered yet: tear down while df is still ours */ + rt_mutex_detach(&sfd->lock); + df->vnode->data = RT_NULL; + rt_free(df->vnode); + df->vnode = RT_NULL; rt_free(sfd); - ret = -1; + fd_release(fd); + return -1; } - sfd->sig_num = 0; - df->flags |= flags; - #ifdef RT_USING_DFS_V2 +#ifdef RT_USING_DFS_V2 df->fops = &signalfd_fops; - #endif +#endif } else { diff --git a/components/lwp/lwp.h b/components/lwp/lwp.h index 49b4324b8daa..f3b2d36afe4f 100644 --- a/components/lwp/lwp.h +++ b/components/lwp/lwp.h @@ -83,6 +83,15 @@ struct rt_lwp_notify * @param signo Signal number */ void (*notify)(rt_wqueue_t *signalfd_queue, int signo); + /** + * @brief Callback invoked when the registered process exits + * @param signalfd_queue Wait queue for signal file descriptors + * @param lwp The exiting process + * @note Called under LWP_LOCK after the node has been unlinked from + * lwp->signalfd_notify_head; the node is freed by the caller + * once this returns. + */ + void (*detach)(rt_wqueue_t *signalfd_queue, struct rt_lwp *lwp); rt_wqueue_t *signalfd_queue; /**< Wait queue for signal file descriptors */ rt_slist_t list_node; /**< List node for notification */ }; diff --git a/components/lwp/lwp_pid.c b/components/lwp/lwp_pid.c index 1f0e09ae9cc8..9089fc7f5cfa 100644 --- a/components/lwp/lwp_pid.c +++ b/components/lwp/lwp_pid.c @@ -2188,6 +2188,7 @@ static void _resr_cleanup(struct rt_lwp *lwp) LWP_LOCK(lwp); lwp_signal_detach(&lwp->signal); + lwp_signal_notify_detach(lwp); /** * @brief Detach children from lwp diff --git a/components/lwp/lwp_signal.c b/components/lwp/lwp_signal.c index 8c88e68151ee..618a0e9c3d76 100644 --- a/components/lwp/lwp_signal.c +++ b/components/lwp/lwp_signal.c @@ -448,6 +448,83 @@ static void lwp_signal_notify(rt_slist_t *list_head, lwp_siginfo_t siginfo) } } +void lwp_signal_notify_add(struct rt_lwp *lwp, struct rt_lwp_notify *notify) +{ + LWP_LOCK(lwp); + rt_slist_append(&lwp->signalfd_notify_head, ¬ify->list_node); + LWP_UNLOCK(lwp); +} + +rt_bool_t lwp_signal_notify_remove(struct rt_lwp *lwp, struct rt_lwp_notify *notify) +{ + rt_slist_t *node; + rt_bool_t found = RT_FALSE; + + LWP_LOCK(lwp); + rt_slist_for_each(node, &lwp->signalfd_notify_head) + { + if (node == ¬ify->list_node) + { + found = RT_TRUE; + break; + } + } + if (found) + { + rt_slist_remove(&lwp->signalfd_notify_head, ¬ify->list_node); + } + LWP_UNLOCK(lwp); + + return found; +} + +void lwp_signal_notify_remove_queue(struct rt_lwp *lwp, rt_wqueue_t *signalfd_queue) +{ + rt_slist_t *prev; + rt_slist_t *curr; + + LWP_LOCK(lwp); + prev = &lwp->signalfd_notify_head; + curr = prev->next; + while (curr != RT_NULL) + { + struct rt_lwp_notify *n = rt_slist_entry(curr, struct rt_lwp_notify, list_node); + + if (n->signalfd_queue == signalfd_queue) + { + prev->next = curr->next; + rt_free(n); + curr = prev->next; + } + else + { + prev = curr; + curr = curr->next; + } + } + LWP_UNLOCK(lwp); +} + +void lwp_signal_notify_detach(struct rt_lwp *lwp) +{ + rt_slist_t *node; + + LWP_ASSERT_LOCKED(lwp); + + while ((node = rt_slist_first(&lwp->signalfd_notify_head)) != RT_NULL) + { + struct rt_lwp_notify *n = rt_slist_entry(node, struct rt_lwp_notify, list_node); + + lwp->signalfd_notify_head.next = node->next; + node->next = RT_NULL; + if (n->detach) + { + n->detach(n->signalfd_queue, lwp); + } + rt_free(n); + } +} + rt_err_t lwp_signal_init(struct lwp_signal *sig) { rt_err_t rc = RT_EOK; diff --git a/components/lwp/lwp_signal.h b/components/lwp/lwp_signal.h index 7e7bbad92050..32e2b22db304 100644 --- a/components/lwp/lwp_signal.h +++ b/components/lwp/lwp_signal.h @@ -96,6 +96,49 @@ rt_err_t lwp_signal_init(struct lwp_signal *sig); rt_err_t lwp_signal_detach(struct lwp_signal *signal); +struct rt_lwp_notify; + +/** + * @brief Register a signal notification listener on a process + * + * @param lwp target process + * @param notify listener node; owned by the process list until it is removed + * + * @note Takes LWP_LOCK, the lock lwp_signal_notify() walks the list under. + */ +void lwp_signal_notify_add(struct rt_lwp *lwp, struct rt_lwp_notify *notify); + +/** + * @brief Unlink one listener node from a process + * + * @param lwp target process + * @param notify listener node to unlink + * + * @return RT_TRUE if the node was on the list and has been unlinked, + * RT_FALSE if it was already gone. The node is not freed. + */ +rt_bool_t lwp_signal_notify_remove(struct rt_lwp *lwp, struct rt_lwp_notify *notify); + +/** + * @brief Unlink and free every listener of a wait queue on a process + * + * @param lwp target process + * @param signalfd_queue wait queue the listeners belong to + * + * @note Takes LWP_LOCK; no notification can be inside a node once it returns. + */ +void lwp_signal_notify_remove_queue(struct rt_lwp *lwp, rt_wqueue_t *signalfd_queue); + +/** + * @brief Release every listener of an exiting process + * + * @param lwp the exiting process + * + * @note Caller holds LWP_LOCK. Each listener's detach callback runs after + * its node is unlinked; the node is freed afterwards. + */ +void lwp_signal_notify_detach(struct rt_lwp *lwp); + rt_inline void lwp_thread_signal_detach(struct lwp_thread_signal *tsig) { lwp_sigqueue_clear(&tsig->sig_queue); From a927d654bfab2537f42d02fc0346f83650e0acad Mon Sep 17 00:00:00 2001 From: Adil Alperen Ciftci Date: Wed, 4 Nov 2026 07:56:12 +0300 Subject: [PATCH 2/5] dfs/posix: state the signalfd context lifetime and tighten close State the lifetime assumption of the signalfd context explicitly: the descriptor the caller operates on pins the context for the whole read, poll and registration, and a concurrent close of that same descriptor by another thread is not supported, which is what the existing read and poll paths already rely on. The post-registration slot re-check therefore does not add a new requirement. Clear all registration slots in one pass under sfd->lock in close and run the per-process unlink and reference drop outside the lock afterwards, so the "clear first, unlink later" invariant is one block instead of a lock round-trip per slot. Rename SIGNALFD_SHART_MAX to SIGNALFD_SHARE_MAX while the macro block is being touched. Signed-off-by: Adil Alperen Ciftci --- components/libc/posix/io/signalfd/signalfd.c | 40 ++++++++++++-------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/components/libc/posix/io/signalfd/signalfd.c b/components/libc/posix/io/signalfd/signalfd.c index 496a3096f031..0d9623681e8f 100644 --- a/components/libc/posix/io/signalfd/signalfd.c +++ b/components/libc/posix/io/signalfd/signalfd.c @@ -21,7 +21,7 @@ #define SIGNALFD_MUTEX_NAME "signalfd" #define SIGINFO_MAX 10 -#define SIGNALFD_SHART_MAX RT_SIGNALFD_MAX_NUM +#define SIGNALFD_SHARE_MAX RT_SIGNALFD_MAX_NUM struct rt_signalfd_ctx { @@ -30,7 +30,7 @@ struct rt_signalfd_ctx siginfo_t info[SIGINFO_MAX]; int sig_num; rt_wqueue_t signalfd_queue; - struct rt_lwp *lwp[SIGNALFD_SHART_MAX]; + struct rt_lwp *lwp[SIGNALFD_SHARE_MAX]; }; static int signalfd_close(struct dfs_file *file); @@ -54,6 +54,12 @@ static int signalfd_add_notify(struct rt_signalfd_ctx *sfd); * Each registered process holds one LWP reference. The slot in sfd->lwp[] is * the registration: whoever clears a slot (close or the process exit hook) * drops that reference, so the pointer stays valid until it is cleared. + * + * The context itself is pinned by the descriptor the caller is operating on: + * read, poll and the registration they perform assume that the same + * descriptor is not closed concurrently by another thread, as the existing + * read/poll paths already do. close() only tears the context down once the + * vnode reference count has dropped to one. */ static const struct dfs_file_ops signalfd_fops = { @@ -85,20 +91,24 @@ static int signalfd_close(struct dfs_file *file) if (sfd) { - for (int i = 0; i < SIGNALFD_SHART_MAX; i++) - { - struct rt_lwp *lwp; + struct rt_lwp *lwps[SIGNALFD_SHARE_MAX]; - rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); - lwp = sfd->lwp[i]; + /* clear every slot first, then unlink the listeners outside sfd->lock */ + rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); + for (int i = 0; i < SIGNALFD_SHARE_MAX; i++) + { + lwps[i] = sfd->lwp[i]; sfd->lwp[i] = RT_NULL; - rt_mutex_release(&sfd->lock); + } + rt_mutex_release(&sfd->lock); - if (lwp) + for (int i = 0; i < SIGNALFD_SHARE_MAX; i++) + { + if (lwps[i]) { - /* the slot held a reference, so lwp is still valid here */ - lwp_signal_notify_remove_queue(lwp, &sfd->signalfd_queue); - lwp_ref_dec(lwp); + /* the slot held a reference, so the process is still valid here */ + lwp_signal_notify_remove_queue(lwps[i], &sfd->signalfd_queue); + lwp_ref_dec(lwps[i]); } } rt_mutex_detach(&sfd->lock); @@ -278,7 +288,7 @@ static void signalfd_detach(rt_wqueue_t *signalfd_queue, struct rt_lwp *lwp) sfd = rt_container_of(signalfd_queue, struct rt_signalfd_ctx, signalfd_queue); rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); - for (int i = 0; i < SIGNALFD_SHART_MAX; i++) + for (int i = 0; i < SIGNALFD_SHARE_MAX; i++) { if (sfd->lwp[i] == lwp) { @@ -307,7 +317,7 @@ static int signalfd_find_slot(struct rt_signalfd_ctx *sfd, struct rt_lwp *lwp, i { *empty_slot = -1; - for (int i = 0; i < SIGNALFD_SHART_MAX; i++) + for (int i = 0; i < SIGNALFD_SHARE_MAX; i++) { if (sfd->lwp[i] == lwp) { @@ -460,7 +470,7 @@ static int signalfd_do(int fd, const sigset_t *mask, int flags) dfs_vnode_init(df->vnode, FT_REGULAR, &signalfd_fops); df->vnode->data = sfd; - for (int i = 0; i < SIGNALFD_SHART_MAX; i++) + for (int i = 0; i < SIGNALFD_SHARE_MAX; i++) { sfd->lwp[i] = RT_NULL; } From c11d465eaed6215c130b450eb4ff84c181800dcc Mon Sep 17 00:00:00 2001 From: Adil Alperen Ciftci Date: Wed, 4 Nov 2026 19:53:21 +0300 Subject: [PATCH 3/5] dfs/posix: keep the LWP lock out of signalfd close and make registration atomic dfs_file_close() calls the signalfd close callback under the global DFS file lock, and the process exit path takes that same lock while it already holds LWP_LOCK (__exit_files() under _resr_cleanup()). Unlinking the listeners from close() through lwp_signal_notify_remove_queue() therefore took the two locks in the opposite order to process exit, and a parent closing a shared signalfd while a registered child exits could block each side on the other's lock. Close no longer takes any LWP lock. It marks the context closing, detaches it from the vnode and submits a release work item to the system workqueue. The work takes the slots over under sfd->lock, unlinks the listeners under each process's own LWP_LOCK with no other lock held, drops the references the slots owned, and frees the context. A process that exits before the work runs has already cleared its slot and dropped its reference through the exit hook, so the work only sees slots that are still live. Registration now does the lookup, the slot assignment and the listener insertion in one critical section under LWP_LOCK(self) -> sfd->lock, the same order the exit hook already uses. A slot is never visible without its listener, so a second thread cannot observe a registration whose listener is still missing, and the lookup cannot go stale between the check and the assignment. lwp_signal_notify_add() accordingly expects the caller to hold LWP_LOCK; the single-node unlink helper that the post-registration re-check used is gone with that re-check. A registration that races with close finds the closing flag and fails with EBADF instead of taking a slot the release work will not see. Signed-off-by: Adil Alperen Ciftci --- components/libc/posix/io/signalfd/signalfd.c | 191 ++++++++++--------- components/lwp/lwp_signal.c | 27 +-- components/lwp/lwp_signal.h | 15 +- 3 files changed, 110 insertions(+), 123 deletions(-) diff --git a/components/libc/posix/io/signalfd/signalfd.c b/components/libc/posix/io/signalfd/signalfd.c index 0d9623681e8f..8516168df2f7 100644 --- a/components/libc/posix/io/signalfd/signalfd.c +++ b/components/libc/posix/io/signalfd/signalfd.c @@ -17,7 +17,9 @@ #include #include #include +#include #include +#include #define SIGNALFD_MUTEX_NAME "signalfd" #define SIGINFO_MAX 10 @@ -31,6 +33,8 @@ struct rt_signalfd_ctx int sig_num; rt_wqueue_t signalfd_queue; struct rt_lwp *lwp[SIGNALFD_SHARE_MAX]; + int closing; /* set by close(); registration refuses afterwards */ + struct rt_work release_work; /* unlinks the listeners and frees the context */ }; static int signalfd_close(struct dfs_file *file); @@ -43,23 +47,33 @@ static ssize_t signalfd_read(struct dfs_file *file, void *buf, size_t count, off static int signalfd_add_notify(struct rt_signalfd_ctx *sfd); /* - * Lock order: LWP_LOCK(lwp) -> sfd->lock. + * Lock order: LWP_LOCK(lwp) -> sfd->lock, never the reverse. * * lwp_signal_notify() walks lwp->signalfd_notify_head under LWP_LOCK and the - * callback takes sfd->lock inside it. The listener list is therefore only - * changed through the lwp_signal_notify_* helpers, which take LWP_LOCK, and - * sfd->lock is never held while calling them. A node is unlinked under - * LWP_LOCK, so no notification can still be inside it when it is freed. + * callback takes sfd->lock inside it. Registration takes LWP_LOCK of the + * calling process and then sfd->lock, and does the lookup, the slot + * assignment and the listener insertion in that one critical section, so a + * slot is never visible without its listener and the lookup cannot go stale + * between the check and the assignment. The process exit hook runs under + * LWP_LOCK and takes sfd->lock the same way. * * Each registered process holds one LWP reference. The slot in sfd->lwp[] is - * the registration: whoever clears a slot (close or the process exit hook) - * drops that reference, so the pointer stays valid until it is cleared. + * the registration: whoever clears a slot (the release work or the process + * exit hook) drops that reference, so the pointer stays valid until cleared. * - * The context itself is pinned by the descriptor the caller is operating on: - * read, poll and the registration they perform assume that the same - * descriptor is not closed concurrently by another thread, as the existing - * read/poll paths already do. close() only tears the context down once the - * vnode reference count has dropped to one. + * close() is called by dfs_file_close() under the global DFS file lock, and + * the process exit path takes that lock while it already holds LWP_LOCK + * (__exit_files() under _resr_cleanup()). close() therefore never takes an + * LWP lock: it marks the context closing, detaches it from the vnode and + * hands the teardown to the system workqueue. The release work unlinks the + * listeners under each process's own LWP_LOCK with no other lock held and + * frees the context afterwards. + * + * The context is pinned by the descriptor the caller is operating on: read, + * poll and the registration they perform assume that the same descriptor is + * not closed concurrently by another thread, as the existing read/poll paths + * already do. close() only tears the context down once the vnode reference + * count has dropped to one. */ static const struct dfs_file_ops signalfd_fops = { @@ -68,10 +82,51 @@ static const struct dfs_file_ops signalfd_fops = { .read = signalfd_read, }; +/** + * @brief Releases a closed signalfd context from the system workqueue. + * @param work The release work embedded in the context. + * @param data Pointer to the signalfd context. + * @note Runs with no lock held, so it may take each registered process's + * LWP_LOCK through lwp_signal_notify_remove_queue(). A process that + * exits before this runs has already cleared its own slot and + * dropped its reference through the exit hook. + */ +static void signalfd_release_work(struct rt_work *work, void *data) +{ + struct rt_signalfd_ctx *sfd = (struct rt_signalfd_ctx *)data; + struct rt_lwp *lwps[SIGNALFD_SHARE_MAX]; + + RT_UNUSED(work); + + /* take the slots over, then unlink the listeners outside sfd->lock */ + rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); + for (int i = 0; i < SIGNALFD_SHARE_MAX; i++) + { + lwps[i] = sfd->lwp[i]; + sfd->lwp[i] = RT_NULL; + } + rt_mutex_release(&sfd->lock); + + for (int i = 0; i < SIGNALFD_SHARE_MAX; i++) + { + if (lwps[i]) + { + /* the slot held a reference, so the process is still valid here */ + lwp_signal_notify_remove_queue(lwps[i], &sfd->signalfd_queue); + lwp_ref_dec(lwps[i]); + } + } + + rt_mutex_detach(&sfd->lock); + rt_free(sfd); +} + /** * @brief Closes the file descriptor associated with a signalfd file. * @param file Pointer to the file structure. * @return Upon successful completion, returns 0; otherwise, returns an error code. + * @note Called under the DFS file lock. Takes only sfd->lock; the listener + * teardown runs later from the system workqueue. */ static int signalfd_close(struct dfs_file *file) { @@ -91,29 +146,14 @@ static int signalfd_close(struct dfs_file *file) if (sfd) { - struct rt_lwp *lwps[SIGNALFD_SHARE_MAX]; - - /* clear every slot first, then unlink the listeners outside sfd->lock */ rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); - for (int i = 0; i < SIGNALFD_SHARE_MAX; i++) - { - lwps[i] = sfd->lwp[i]; - sfd->lwp[i] = RT_NULL; - } + sfd->closing = 1; rt_mutex_release(&sfd->lock); - for (int i = 0; i < SIGNALFD_SHARE_MAX; i++) - { - if (lwps[i]) - { - /* the slot held a reference, so the process is still valid here */ - lwp_signal_notify_remove_queue(lwps[i], &sfd->signalfd_queue); - lwp_ref_dec(lwps[i]); - } - } - rt_mutex_detach(&sfd->lock); file->vnode->data = RT_NULL; - rt_free(sfd); + + rt_work_init(&sfd->release_work, signalfd_release_work, sfd); + rt_work_submit(&sfd->release_work, 0); } return 0; @@ -333,17 +373,20 @@ static int signalfd_find_slot(struct rt_signalfd_ctx *sfd, struct rt_lwp *lwp, i } /** - * @brief Adds a signal file descriptor notification. + * @brief Registers the calling process on a signalfd context. * @param sfd Pointer to the signalfd context. * @return Upon successful completion, returns 0; otherwise, returns an error code. + * @note Lookup, slot assignment and listener insertion happen in one + * critical section under LWP_LOCK(self) -> sfd->lock, so another + * thread can only see the slot once the listener is installed. */ static int signalfd_add_notify(struct rt_signalfd_ctx *sfd) { struct rt_lwp_notify *lwp_notify; - rt_err_t ret = 0; + struct rt_lwp *curr_lwp; int registered; int empty_slot; - struct rt_lwp *curr_lwp; + int err = 0; if (!sfd) { @@ -356,21 +399,7 @@ static int signalfd_add_notify(struct rt_signalfd_ctx *sfd) return -EINVAL; } - rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); - registered = signalfd_find_slot(sfd, curr_lwp, &empty_slot); - rt_mutex_release(&sfd->lock); - - if (registered) - { - return 0; - } - - if (empty_slot < 0) - { - rt_set_errno(EMFILE); - return -EMFILE; - } - + /* allocate before taking any lock; freed again if no slot is taken */ lwp_notify = (struct rt_lwp_notify *)rt_malloc(sizeof(struct rt_lwp_notify)); if (!lwp_notify) { @@ -381,58 +410,47 @@ static int signalfd_add_notify(struct rt_signalfd_ctx *sfd) lwp_notify->detach = signalfd_detach; lwp_notify->signalfd_queue = &sfd->signalfd_queue; - /* the slot owns this reference until close or process exit clears it */ - lwp_ref_inc(curr_lwp); - + LWP_LOCK(curr_lwp); rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); - if (sfd->lwp[empty_slot] != RT_NULL) - { - /* lost the slot meanwhile: look again */ - registered = signalfd_find_slot(sfd, curr_lwp, &empty_slot); - } - if (!registered && empty_slot >= 0) + + if (sfd->closing) { - sfd->lwp[empty_slot] = curr_lwp; + err = -EBADF; } - rt_mutex_release(&sfd->lock); - - if (registered || empty_slot < 0) + else { - lwp_ref_dec(curr_lwp); - rt_free(lwp_notify); - if (registered) + registered = signalfd_find_slot(sfd, curr_lwp, &empty_slot); + if (!registered) { - return 0; + if (empty_slot < 0) + { + err = -EMFILE; + } + else + { + /* the slot owns this reference until the release work or the exit hook clears it */ + lwp_ref_inc(curr_lwp); + sfd->lwp[empty_slot] = curr_lwp; + lwp_signal_notify_add(curr_lwp, lwp_notify); + lwp_notify = RT_NULL; + } } - rt_set_errno(EMFILE); - return -EMFILE; } - lwp_signal_notify_add(curr_lwp, lwp_notify); + rt_mutex_release(&sfd->lock); + LWP_UNLOCK(curr_lwp); - /* - * close() clears the slot first and unlinks listeners afterwards. If the - * slot is gone now, close is running and may already have walked the - * list; take the node back ourselves if it is still there. The reference - * was dropped by whoever cleared the slot. - */ - rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); - if (sfd->lwp[empty_slot] != curr_lwp) + if (lwp_notify) { - ret = -EBADF; + rt_free(lwp_notify); } - rt_mutex_release(&sfd->lock); - if (ret < 0) + if (err < 0) { - if (lwp_signal_notify_remove(curr_lwp, lwp_notify)) - { - rt_free(lwp_notify); - } - rt_set_errno(EBADF); + rt_set_errno(-err); } - return ret; + return err; } /** @@ -479,6 +497,7 @@ static int signalfd_do(int fd, const sigset_t *mask, int flags) memcpy(&sfd->sigmask, mask, sizeof(sigset_t)); sfd->sig_num = 0; + sfd->closing = 0; rt_mutex_init(&sfd->lock, SIGNALFD_MUTEX_NAME, RT_IPC_FLAG_FIFO); rt_wqueue_init(&sfd->signalfd_queue); diff --git a/components/lwp/lwp_signal.c b/components/lwp/lwp_signal.c index 618a0e9c3d76..771655bc949a 100644 --- a/components/lwp/lwp_signal.c +++ b/components/lwp/lwp_signal.c @@ -450,32 +450,9 @@ static void lwp_signal_notify(rt_slist_t *list_head, lwp_siginfo_t siginfo) void lwp_signal_notify_add(struct rt_lwp *lwp, struct rt_lwp_notify *notify) { - LWP_LOCK(lwp); - rt_slist_append(&lwp->signalfd_notify_head, ¬ify->list_node); - LWP_UNLOCK(lwp); -} - -rt_bool_t lwp_signal_notify_remove(struct rt_lwp *lwp, struct rt_lwp_notify *notify) -{ - rt_slist_t *node; - rt_bool_t found = RT_FALSE; - - LWP_LOCK(lwp); - rt_slist_for_each(node, &lwp->signalfd_notify_head) - { - if (node == ¬ify->list_node) - { - found = RT_TRUE; - break; - } - } - if (found) - { - rt_slist_remove(&lwp->signalfd_notify_head, ¬ify->list_node); - } - LWP_UNLOCK(lwp); + LWP_ASSERT_LOCKED(lwp); - return found; + rt_slist_append(&lwp->signalfd_notify_head, ¬ify->list_node); } void lwp_signal_notify_remove_queue(struct rt_lwp *lwp, rt_wqueue_t *signalfd_queue) diff --git a/components/lwp/lwp_signal.h b/components/lwp/lwp_signal.h index 32e2b22db304..d915d8578424 100644 --- a/components/lwp/lwp_signal.h +++ b/components/lwp/lwp_signal.h @@ -104,21 +104,12 @@ struct rt_lwp_notify; * @param lwp target process * @param notify listener node; owned by the process list until it is removed * - * @note Takes LWP_LOCK, the lock lwp_signal_notify() walks the list under. + * @note Caller holds LWP_LOCK, the lock lwp_signal_notify() walks the list + * under, so the caller can publish its own registration state in the + * same critical section. */ void lwp_signal_notify_add(struct rt_lwp *lwp, struct rt_lwp_notify *notify); -/** - * @brief Unlink one listener node from a process - * - * @param lwp target process - * @param notify listener node to unlink - * - * @return RT_TRUE if the node was on the list and has been unlinked, - * RT_FALSE if it was already gone. The node is not freed. - */ -rt_bool_t lwp_signal_notify_remove(struct rt_lwp *lwp, struct rt_lwp_notify *notify); - /** * @brief Unlink and free every listener of a wait queue on a process * From a2a624e95030d193e649bd5c54db8345510d7897 Mon Sep 17 00:00:00 2001 From: Adil Alperen Ciftci Date: Thu, 5 Nov 2026 05:14:07 +0300 Subject: [PATCH 4/5] dfs/posix: do not allocate for an existing signalfd registration signalfd_add_notify() allocated the listener node before looking the calling process up, so read and poll on an already registered descriptor failed with ENOMEM or POLLERR under memory pressure even though no node was needed and a signal may already have been queued. Look the process up first, under LWP_LOCK(self) -> sfd->lock, and return from there when it is registered or the context is closing. Only when a new registration is needed drop the locks, allocate the node, take the locks again and repeat the lookup unconditionally: the closing flag, an existing registration and the free slot are all re-evaluated before the slot is assigned and the listener is linked in the same critical section. A node left over because another thread registered the process meanwhile is freed after the locks are released. Signed-off-by: Adil Alperen Ciftci --- components/libc/posix/io/signalfd/signalfd.c | 77 ++++++++++++-------- 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/components/libc/posix/io/signalfd/signalfd.c b/components/libc/posix/io/signalfd/signalfd.c index 8516168df2f7..6d445a714388 100644 --- a/components/libc/posix/io/signalfd/signalfd.c +++ b/components/libc/posix/io/signalfd/signalfd.c @@ -378,11 +378,14 @@ static int signalfd_find_slot(struct rt_signalfd_ctx *sfd, struct rt_lwp *lwp, i * @return Upon successful completion, returns 0; otherwise, returns an error code. * @note Lookup, slot assignment and listener insertion happen in one * critical section under LWP_LOCK(self) -> sfd->lock, so another - * thread can only see the slot once the listener is installed. + * thread can only see the slot once the listener is installed. An + * existing registration is answered from that first lookup without + * allocating; a node is only allocated when a new registration is + * needed, and the lookup is repeated after the allocation. */ static int signalfd_add_notify(struct rt_signalfd_ctx *sfd) { - struct rt_lwp_notify *lwp_notify; + struct rt_lwp_notify *lwp_notify = RT_NULL; struct rt_lwp *curr_lwp; int registered; int empty_slot; @@ -399,42 +402,51 @@ static int signalfd_add_notify(struct rt_signalfd_ctx *sfd) return -EINVAL; } - /* allocate before taking any lock; freed again if no slot is taken */ - lwp_notify = (struct rt_lwp_notify *)rt_malloc(sizeof(struct rt_lwp_notify)); - if (!lwp_notify) + for (;;) { - rt_set_errno(ENOMEM); - return -ENOMEM; - } - lwp_notify->notify = signalfd_callback; - lwp_notify->detach = signalfd_detach; - lwp_notify->signalfd_queue = &sfd->signalfd_queue; + LWP_LOCK(curr_lwp); + rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); - LWP_LOCK(curr_lwp); - rt_mutex_take(&sfd->lock, RT_WAITING_FOREVER); + if (sfd->closing) + { + err = -EBADF; + break; + } - if (sfd->closing) - { - err = -EBADF; - } - else - { registered = signalfd_find_slot(sfd, curr_lwp, &empty_slot); - if (!registered) + if (registered) { - if (empty_slot < 0) - { - err = -EMFILE; - } - else - { - /* the slot owns this reference until the release work or the exit hook clears it */ - lwp_ref_inc(curr_lwp); - sfd->lwp[empty_slot] = curr_lwp; - lwp_signal_notify_add(curr_lwp, lwp_notify); - lwp_notify = RT_NULL; - } + /* already registered: nothing to allocate or link */ + break; + } + if (empty_slot < 0) + { + err = -EMFILE; + break; + } + if (lwp_notify) + { + /* the slot owns this reference until the release work or the exit hook clears it */ + lwp_ref_inc(curr_lwp); + sfd->lwp[empty_slot] = curr_lwp; + lwp_signal_notify_add(curr_lwp, lwp_notify); + lwp_notify = RT_NULL; + break; + } + + /* a new registration is needed: allocate outside the locks and look again */ + rt_mutex_release(&sfd->lock); + LWP_UNLOCK(curr_lwp); + + lwp_notify = (struct rt_lwp_notify *)rt_malloc(sizeof(struct rt_lwp_notify)); + if (!lwp_notify) + { + rt_set_errno(ENOMEM); + return -ENOMEM; } + lwp_notify->notify = signalfd_callback; + lwp_notify->detach = signalfd_detach; + lwp_notify->signalfd_queue = &sfd->signalfd_queue; } rt_mutex_release(&sfd->lock); @@ -442,6 +454,7 @@ static int signalfd_add_notify(struct rt_signalfd_ctx *sfd) if (lwp_notify) { + /* the process got registered meanwhile, or the context is closing */ rt_free(lwp_notify); } From b4f069e0d183c39ba7e9e512a125ff0beedb8432 Mon Sep 17 00:00:00 2001 From: Adil Alperen Ciftci Date: Thu, 5 Nov 2026 08:32:06 +0300 Subject: [PATCH 5/5] dfs/posix: align signalfd context member comments for clang-format Signed-off-by: Adil Alperen Ciftci --- components/libc/posix/io/signalfd/signalfd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/libc/posix/io/signalfd/signalfd.c b/components/libc/posix/io/signalfd/signalfd.c index 6d445a714388..10bdd395ce85 100644 --- a/components/libc/posix/io/signalfd/signalfd.c +++ b/components/libc/posix/io/signalfd/signalfd.c @@ -33,8 +33,8 @@ struct rt_signalfd_ctx int sig_num; rt_wqueue_t signalfd_queue; struct rt_lwp *lwp[SIGNALFD_SHARE_MAX]; - int closing; /* set by close(); registration refuses afterwards */ - struct rt_work release_work; /* unlinks the listeners and frees the context */ + int closing; /* set by close(); registration refuses afterwards */ + struct rt_work release_work; /* unlinks the listeners and frees the context */ }; static int signalfd_close(struct dfs_file *file);