zzby0 commented on code in PR #9923:
URL: https://github.com/apache/nuttx/pull/9923#discussion_r1278979939


##########
sched/signal/sig_procmask.c:
##########
@@ -187,8 +187,27 @@ int nxsig_procmask(int how, FAR const sigset_t *set, FAR 
sigset_t *oset)
 
 int sigprocmask(int how, FAR const sigset_t *set, FAR sigset_t *oset)
 {
+  sigset_t nset;
   int ret;
 
+  /* SIGKILL and SIGSTOP should not be added to signal mask */
+
+  if (set != NULL)
+    {
+      nset = *set;
+      if (nxsig_ismember(&nset, SIGKILL))
+        {
+          nxsig_delset(&nset, SIGKILL);
+        }
+
+      if (nxsig_ismember(&nset, SIGSTOP))
+        {
+          nxsig_delset(&nset, SIGSTOP);
+        }
+

Review Comment:
   > In the event of an error in some other part of the signal handling logic, 
if SIGKILL or SIGSTOP ever get set in the sigprocmask, they may never be 
cleared. They can be cleared with SIG_SETMASK but that is not often used after 
initialization.
   > 
   > Wouldn't it be safer and more robust to mask out SIGKILL and SIGSTOP 
_after_ the new sigprocmaks (or pthread sigprocmask) is set.
   
   Where the new sigprocmask is set? Should I change nxsig_procmask()? I find 
some functions directly call nxsig_procmask() rather than sig_procmask() or 
pthread_procmask(). But ... we shouldn't limit kernel function to handle 
sigstop/sigkill, right?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to