apache/nuttx
1
0
Fork 0
mirror of https://github.com/apache/nuttx.git synced 2025-05-08 22:32:04 +08:00
Code Issues Packages Projects Releases Wiki Activity

sched/signal/sig_dispatch: Fix race conditions between nxsig_tcbdispatch and nxsig_deliver

Browse Source 
For example a race in managing tcb->sigprocmask may cause signal not being delivered at all.
The mechanism is as follows:

1. cpu 1: nxsig_deliver adds the signal to the sigprocmask for the duration of signal delivery
2. cpu 2: new signal (same signo) is dispatched in nxsig_tcbdispatch
3. cpu 2: nxsig_tcbdispatch detects that signal is masked
4. cpu 2: nxsig_tcbdispatch leaves critical section before calling nxsig_add_pendigsignal
5. cpu 1: nxsig_deliver finishes. The "nxsig_unmask_pendingsignal" is called in the end but does nothing
6. cpu 2: nxsig_tcbdispatch continues and adds the pending signal

In the end, the logic in the end of nxsig_deliver, which tries to handle signals added
to the pending queue during the signal action delivery (step 5) failed, and the pending signal
was not delivered.

Fix this by just keeping the critical section for during the whole duration of nxsig_tcbdispatch,
and move things which can't be executed from within critical section outside.

Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
This commit is contained in:
Jukka Laitinen 2025-05-02 10:28:47 +03:00 committed by Alan C. Assis
parent ed0c18c66c
commit 2b513af2c3
1 changed files with 29 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
sched/signal/sig_dispatch.c
View File

@ -105,13 +105,15 @@ static int sig_handler(FAR void *cookie)
* Returned Value:
* Returns 0 (OK) on success or a negated errno value on failure.
*
* Assumptions:
* Called in critical section
*
****************************************************************************/
static int nxsig_queue_action(FAR struct tcb_s *stcb, siginfo_t *info)
{
FAR sigactq_t *sigact;
FAR sigq_t *sigq;
irqstate_t flags;
int ret = OK;
DEBUGASSERT(stcb != NULL && stcb->group != NULL);
@ -152,7 +154,6 @@ static int nxsig_queue_action(FAR struct tcb_s *stcb, siginfo_t *info)
/* Put it at the end of the pending signals list */
flags = enter_critical_section();
sq_addlast((FAR sq_entry_t *)sigq, &(stcb->sigpendactionq));
/* Then schedule execution of the signal handling action on the
@ -195,8 +196,6 @@ static int nxsig_queue_action(FAR struct tcb_s *stcb, siginfo_t *info)
up_schedule_sigaction(stcb);
}
}
leave_critical_section(flags);
}
}
@ -209,12 +208,14 @@ static int nxsig_queue_action(FAR struct tcb_s *stcb, siginfo_t *info)
* Description:
* Allocate a pending signal list entry
*
* Assumptions:
* Called with g_sigpendingsignal locked
*
****************************************************************************/
static FAR sigpendq_t *nxsig_alloc_pendingsignal(void)
{
FAR sigpendq_t *sigpend;
irqstate_t flags;
/* Check if we were called from an interrupt handler. */
@ -242,9 +243,7 @@ static FAR sigpendq_t *nxsig_alloc_pendingsignal(void)
{
/* Try to get the pending signal structure from the free list */
flags = enter_critical_section();
sigpend = (FAR sigpendq_t *)sq_remfirst(&g_sigpendingsignal);
leave_critical_section(flags);
/* Check if we got one. */
@ -272,13 +271,15 @@ static FAR sigpendq_t *nxsig_alloc_pendingsignal(void)
* Description:
* Find a specified element in the pending signal list
*
* Assumptions:
* Called with group->tg_sigpendingq locked
*
****************************************************************************/
static FAR sigpendq_t *
nxsig_find_pendingsignal(FAR struct task_group_s *group, int signo)
{
FAR sigpendq_t *sigpend = NULL;
irqstate_t flags;
DEBUGASSERT(group != NULL);
@ -289,17 +290,12 @@ nxsig_find_pendingsignal(FAR struct task_group_s *group, int signo)
return sigpend;
}
/* Pending signals can be added from interrupt level. */
flags = enter_critical_section();
/* Search the list for a action pending on this signal */
for (sigpend = (FAR sigpendq_t *)group->tg_sigpendingq.head;
(sigpend && sigpend->info.si_signo != signo);
sigpend = sigpend->flink);
leave_critical_section(flags);
return sigpend;
}
@ -330,14 +326,16 @@ static void nxsig_dispatch_kernel_action(FAR struct tcb_s *stcb,
* was done intentionally so that a run-away sender cannot consume
* all of memory.
*
* Assumptions:
* Called with tg_sigpendingq locked
*
****************************************************************************/
static void nxsig_add_pendingsignal(FAR struct tcb_s *stcb,
FAR siginfo_t *info)
static FAR sigpendq_t *nxsig_add_pendingsignal(FAR struct tcb_s *stcb,
FAR siginfo_t *info)
{
FAR struct task_group_s *group;
FAR sigpendq_t *sigpend;
irqstate_t flags;
DEBUGASSERT(stcb != NULL && stcb->group != NULL);
group = stcb->group;
@ -367,14 +365,13 @@ static void nxsig_add_pendingsignal(FAR struct tcb_s *stcb,
/* Add the structure to the group pending signal list */
flags = enter_critical_section();
sq_addlast((FAR sq_entry_t *)sigpend, &group->tg_sigpendingq);
leave_critical_section(flags);
nxsig_dispatch_kernel_action(stcb, &sigpend->info);
}
}
DEBUGASSERT(sigpend);
return sigpend;
}
/****************************************************************************
@ -409,6 +406,7 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info)
irqstate_t flags;
int masked;
int ret = OK;
FAR sigpendq_t *sigpend = NULL;
sinfo("TCB=%p pid=%d signo=%d code=%d value=%d masked=%s\n",
stcb, stcb->pid, info->si_signo, info->si_code,
@ -433,6 +431,8 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info)
/************************** MASKED SIGNAL ACTIONS *************************/
flags = enter_critical_section();
masked = nxsig_ismember(&stcb->sigprocmask, info->si_signo);
#ifdef CONFIG_LIB_SYSCALL
@ -474,7 +474,6 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info)
* signals can be queued from the interrupt level.
*/
flags = enter_critical_section();
if (stcb->task_state == TSTATE_WAIT_SIG &&
(masked == 0 ||
nxsig_ismember(&stcb->sigwaitmask, info->si_signo)))
@ -504,14 +503,12 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info)
up_switch_context(stcb, rtcb);
}
leave_critical_section(flags);
#ifdef CONFIG_LIB_SYSCALL
/* Must also add signal action if in system call */
if (masked == 0)
{
nxsig_add_pendingsignal(stcb, info);
sigpend = nxsig_add_pendingsignal(stcb, info);
}
#endif
}
@ -522,8 +519,7 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info)
else
{
leave_critical_section(flags);
nxsig_add_pendingsignal(stcb, info);
sigpend = nxsig_add_pendingsignal(stcb, info);
}
}
@ -537,8 +533,6 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info)
/* Deliver of the signal must be performed in a critical section */
flags = enter_critical_section();
/* Check if the task is waiting for an unmasked signal. If so, then
* unblock it. This must be performed in a critical section because
* signals can be queued from the interrupt level.
@ -572,8 +566,6 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info)
}
}
leave_critical_section(flags);
/* If the task neither was waiting for the signal nor had a signal
* handler attached to the signal, then the default action is
* simply to ignore the signal
@ -588,8 +580,6 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info)
if (masked == 0)
{
flags = enter_critical_section();
/* If the task is blocked waiting for a semaphore, then that task must
* be unblocked when a signal is received.
*/
@ -637,8 +627,15 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info)
#endif
}
#endif
}
leave_critical_section(flags);
leave_critical_section(flags);
/* Dispatch kernel action, if needed, in case a pending signal was added */
if (sigpend != NULL)
{
nxsig_dispatch_kernel_action(stcb, &sigpend->info);
}
/* In case nxsig_ismember failed due to an invalid signal number */