p_sigmask is only modified by the owning thread from process context
(e.g., sys_sigprocmask(), sys_sigreturn(), userret(), or postsig()),
but it can be accessed anywhere (e.g., interrupts or threads on other
CPUs). Currently sys_sigprocmask() protects p_sigmask with splhigh(),
but that's not MP-safe.
The simpler solution is to take advantage of our atomics APIs.
Unfortunately for implementing SIG_SETMASK, we don't have an
atomic_store_int() function, and I can't bring myself to abuse
"volatile" for this purpose, so I've resorted to atomic_swap_uint().
While here, I also refactored the P_SIGSUSPEND code a bit so there's
less code duplication.
I've included just amd64 and i386's machdep.c for now, because those
were the only ones I'm readily able to test. The others should be
easy to similarly fix though, and I can send a followup diff for them.
ok?
Index: sys/proc.h
===================================================================
RCS file: /home/matthew/cvs-mirror/cvs/src/sys/sys/proc.h,v
retrieving revision 1.186
diff -u -p -r1.186 proc.h
--- sys/proc.h 15 May 2014 03:52:25 -0000 1.186
+++ sys/proc.h 7 Jul 2014 19:23:06 -0000
@@ -520,6 +520,8 @@ int proc_cansugid(struct proc *);
void proc_finish_wait(struct proc *, struct proc *);
void process_zap(struct process *);
void proc_free(struct proc *);
+void proc_sigsetmask(struct proc *, sigset_t);
+void proc_sigsuspend(struct proc *, sigset_t);
struct sleep_state {
int sls_s;
@@ -559,4 +561,3 @@ struct cpu_info *cpuset_first(struct cpu
#endif /* _KERNEL */
#endif /* !_SYS_PROC_H_ */
-
Index: kern/kern_sig.c
===================================================================
RCS file: /home/matthew/cvs-mirror/cvs/src/sys/kern/kern_sig.c,v
retrieving revision 1.166
diff -u -p -r1.166 kern_sig.c
--- kern/kern_sig.c 4 May 2014 05:03:26 -0000 1.166
+++ kern/kern_sig.c 7 Jul 2014 19:24:07 -0000
@@ -62,6 +62,7 @@
#include <sys/ptrace.h>
#include <sys/sched.h>
#include <sys/user.h>
+#include <sys/atomic.h>
#include <sys/mount.h>
#include <sys/syscallargs.h>
@@ -439,37 +440,55 @@ sys_sigprocmask(struct proc *p, void *v,
syscallarg(int) how;
syscallarg(sigset_t) mask;
} */ *uap = v;
- int error = 0;
- int s;
- sigset_t mask;
+ sigset_t mask = SCARG(uap, mask);
*retval = p->p_sigmask;
- mask = SCARG(uap, mask);
- s = splhigh();
switch (SCARG(uap, how)) {
case SIG_BLOCK:
- p->p_sigmask |= mask &~ sigcantmask;
+ atomic_setbits_int(&p->p_sigmask, mask &~ sigcantmask);
break;
case SIG_UNBLOCK:
- p->p_sigmask &= ~mask;
+ atomic_clearbits_int(&p->p_sigmask, mask);
break;
case SIG_SETMASK:
- p->p_sigmask = mask &~ sigcantmask;
+ proc_sigsetmask(p, mask &~ sigcantmask);
break;
default:
- error = EINVAL;
- break;
+ return (EINVAL);
}
- splx(s);
- return (error);
+
+ return (0);
+}
+
+void
+proc_sigsetmask(struct proc *p, sigset_t mask)
+{
+ KASSERT(p == curproc);
+
+ /* XXX: An atomic store would suffice here. */
+ (void)atomic_swap_uint(&p->p_sigmask, mask);
+}
+
+/*
+ * Temporarily replace calling proc's signal mask for the duration of a
+ * system call. Original signal mask will be restored by userret().
+ */
+void
+proc_sigsuspend(struct proc *p, sigset_t mask)
+{
+ KASSERT(p == curproc);
+ KASSERT(!(p->p_flag & P_SIGSUSPEND));
+
+ p->p_oldmask = p->p_sigmask;
+ atomic_setbits_int(&p->p_flag, P_SIGSUSPEND);
+ proc_sigsetmask(p, mask);
}
/* ARGSUSED */
int
sys_sigpending(struct proc *p, void *v, register_t *retval)
{
-
*retval = p->p_siglist;
return (0);
}
@@ -489,16 +508,7 @@ sys_sigsuspend(struct proc *p, void *v,
struct process *pr = p->p_p;
struct sigacts *ps = pr->ps_sigacts;
- /*
- * When returning from sigpause, we want
- * the old mask to be restored after the
- * signal handler has finished. Thus, we
- * save it here and mark the sigacts structure
- * to indicate this.
- */
- p->p_oldmask = p->p_sigmask;
- atomic_setbits_int(&p->p_flag, P_SIGSUSPEND);
- p->p_sigmask = SCARG(uap, mask) &~ sigcantmask;
+ proc_sigsuspend(p, SCARG(uap, mask) &~ sigcantmask);
while (tsleep(ps, PPAUSE|PCATCH, "pause", 0) == 0)
/* void */;
/* always return EINTR rather than ERESTART... */
Index: kern/sys_generic.c
===================================================================
RCS file: /home/matthew/cvs-mirror/cvs/src/sys/kern/sys_generic.c,v
retrieving revision 1.86
diff -u -p -r1.86 sys_generic.c
--- kern/sys_generic.c 30 Mar 2014 21:54:48 -0000 1.86
+++ kern/sys_generic.c 7 Jul 2014 19:24:07 -0000
@@ -658,9 +620,7 @@ dopselect(struct proc *p, int nd, fd_set
timo = 0;
if (sigmask) {
- p->p_oldmask = p->p_sigmask;
- atomic_setbits_int(&p->p_flag, P_SIGSUSPEND);
- p->p_sigmask = *sigmask &~ sigcantmask;
+ proc_sigsuspend(p, *sigmask &~ sigcantmask);
}
retry:
@@ -966,9 +926,7 @@ doppoll(struct proc *p, struct pollfd *f
timo = 0;
if (sigmask) {
- p->p_oldmask = p->p_sigmask;
- atomic_setbits_int(&p->p_flag, P_SIGSUSPEND);
- p->p_sigmask = *sigmask &~ sigcantmask;
+ proc_sigsuspend(p, *sigmask &~ sigcantmask);
}
retry:
Index: compat/linux/linux_signal.c
===================================================================
RCS file: /home/matthew/cvs-mirror/cvs/src/sys/compat/linux/linux_signal.c,v
retrieving revision 1.17
diff -u -p -r1.17 linux_signal.c
--- compat/linux/linux_signal.c 26 Mar 2014 05:23:42 -0000 1.17
+++ compat/linux/linux_signal.c 7 Jul 2014 19:24:07 -0000
@@ -584,7 +584,6 @@ linux_sys_sigprocmask(p, v, retval)
linux_old_sigset_t ss;
sigset_t bs;
int error = 0;
- int s;
*retval = 0;
@@ -604,19 +603,17 @@ linux_sys_sigprocmask(p, v, retval)
linux_old_to_bsd_sigset(&ss, &bs);
- s = splhigh();
-
switch (SCARG(uap, how)) {
case LINUX_SIG_BLOCK:
- p->p_sigmask |= bs & ~sigcantmask;
+ atomic_setbits_int(&p->p_sigmask, bs &~ sigcantmask);
break;
case LINUX_SIG_UNBLOCK:
- p->p_sigmask &= ~bs;
+ atomic_clearbits_int(&p->p_sigmask, bs);
break;
case LINUX_SIG_SETMASK:
- p->p_sigmask = bs & ~sigcantmask;
+ proc_sigsetmask(p, bs &~ sigcantmask);
break;
default:
@@ -624,8 +621,6 @@ linux_sys_sigprocmask(p, v, retval)
break;
}
- splx(s);
-
return (error);
}
@@ -644,7 +639,6 @@ linux_sys_rt_sigprocmask(p, v, retval)
linux_sigset_t ls;
sigset_t bs;
int error = 0;
- int s;
if (SCARG(uap, sigsetsize) != sizeof(linux_sigset_t))
return (EINVAL);
@@ -667,19 +661,17 @@ linux_sys_rt_sigprocmask(p, v, retval)
linux_to_bsd_sigset(&ls, &bs);
- s = splhigh();
-
switch (SCARG(uap, how)) {
case LINUX_SIG_BLOCK:
- p->p_sigmask |= bs & ~sigcantmask;
+ atomic_setbits_int(&p->p_sigmask, bs &~ sigcantmask);
break;
case LINUX_SIG_UNBLOCK:
- p->p_sigmask &= ~bs;
+ atomic_clearbits_int(&p->p_sigmask, bs);
break;
case LINUX_SIG_SETMASK:
- p->p_sigmask = bs & ~sigcantmask;
+ proc_sigsetmask(p, bs &~ sigcantmask);
break;
default:
@@ -687,8 +679,6 @@ linux_sys_rt_sigprocmask(p, v, retval)
break;
}
- splx(s);
-
return (error);
}
@@ -727,16 +717,13 @@ linux_sys_sigsetmask(p, v, retval)
} */ *uap = v;
linux_old_sigset_t mask;
sigset_t bsdsig;
- int s;
bsd_to_linux_old_sigset(&p->p_sigmask, (linux_old_sigset_t *)retval);
mask = SCARG(uap, mask);
bsd_to_linux_old_sigset(&bsdsig, &mask);
- s = splhigh();
- p->p_sigmask = bsdsig & ~sigcantmask;
- splx(s);
+ proc_sigsetmask(p, bsdsig &~ sigcantmask);
return (0);
}
Index: arch/amd64/amd64/machdep.c
===================================================================
RCS file: /home/matthew/cvs-mirror/cvs/src/sys/arch/amd64/amd64/machdep.c,v
retrieving revision 1.180
diff -u -p -r1.180 machdep.c
--- arch/amd64/amd64/machdep.c 15 Jun 2014 11:43:24 -0000 1.180
+++ arch/amd64/amd64/machdep.c 7 Jul 2014 19:24:07 -0000
@@ -670,7 +670,7 @@ sys_sigreturn(struct proc *p, void *v, r
bcopy(&ksc, tf, sizeof(*tf));
/* Restore signal mask. */
- p->p_sigmask = ksc.sc_mask & ~sigcantmask;
+ proc_sigsetmask(p, ksc.sc_mask & ~sigcantmask);
/*
* sigreturn() needs to return to userspace via the 'iretq'
Index: arch/i386/i386/machdep.c
===================================================================
RCS file: /home/matthew/cvs-mirror/cvs/src/sys/arch/i386/i386/machdep.c,v
retrieving revision 1.539
diff -u -p -r1.539 machdep.c
--- arch/i386/i386/machdep.c 15 Jun 2014 11:43:24 -0000 1.539
+++ arch/i386/i386/machdep.c 7 Jul 2014 19:24:07 -0000
@@ -2491,7 +2491,7 @@ sys_sigreturn(struct proc *p, void *v, r
p->p_md.md_flags |= MDP_USEDFPU;
}
- p->p_sigmask = context.sc_mask & ~sigcantmask;
+ proc_sigsetmask(p, context.sc_mask & ~sigcantmask);
return (EJUSTRETURN);
}
@@ -3990,4 +3990,3 @@ intr_handler(struct intrframe *frame, st
#endif
return rc;
}
-
