The TIF_RESTORE_SIGMASK flag allows us to have a generic implementation
of sys_rt_sigsuspend() instead of duplicating it for each architecture.
This provides such an implementation and makes arch/powerpc use it.
It also tidies up the ppc32 sys_sigsuspend() to use TIF_RESTORE_SIGMASK.
Signed-off-by: David Woodhouse <[email protected]>
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -252,8 +252,7 @@ int do_signal(sigset_t *oldset, struct p
/*
* Atomically swap in the new signal mask, and wait for a signal.
*/
-long sys_sigsuspend(old_sigset_t mask, int p2, int p3, int p4, int p6, int p7,
- struct pt_regs *regs)
+long sys_sigsuspend(old_sigset_t mask)
{
sigset_t saveset;
@@ -264,55 +263,10 @@ long sys_sigsuspend(old_sigset_t mask, i
recalc_sigpending();
spin_unlock_irq(¤t->sighand->siglock);
- regs->result = -EINTR;
- regs->gpr[3] = EINTR;
- regs->ccr |= 0x10000000;
- while (1) {
- current->state = TASK_INTERRUPTIBLE;
- schedule();
- if (do_signal(&saveset, regs)) {
- set_thread_flag(TIF_RESTOREALL);
- return 0;
- }
- }
-}
-
-long sys_rt_sigsuspend(
-#ifdef CONFIG_PPC64
- compat_sigset_t __user *unewset,
-#else
- sigset_t __user *unewset,
-#endif
- size_t sigsetsize, int p3, int p4,
- int p6, int p7, struct pt_regs *regs)
-{
- sigset_t saveset, newset;
-
- /* XXX: Don't preclude handling different sized sigset_t's. */
- if (sigsetsize != sizeof(sigset_t))
- return -EINVAL;
-
- if (get_sigset_t(&newset, unewset))
- return -EFAULT;
- sigdelsetmask(&newset, ~_BLOCKABLE);
-
- spin_lock_irq(¤t->sighand->siglock);
- saveset = current->blocked;
- current->blocked = newset;
- recalc_sigpending();
- spin_unlock_irq(¤t->sighand->siglock);
-
- regs->result = -EINTR;
- regs->gpr[3] = EINTR;
- regs->ccr |= 0x10000000;
- while (1) {
- current->state = TASK_INTERRUPTIBLE;
- schedule();
- if (do_signal(&saveset, regs)) {
- set_thread_flag(TIF_RESTOREALL);
- return 0;
- }
- }
+ current->state = TASK_INTERRUPTIBLE;
+ schedule();
+ set_thread_flag(TIF_RESTORE_SIGMASK);
+ return -ERESTARTNOHAND;
}
#ifdef CONFIG_PPC32
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 5462bef..7a6f0f7 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -67,42 +67,6 @@ struct rt_sigframe {
char abigap[288];
} __attribute__ ((aligned (16)));
-
-/*
- * Atomically swap in the new signal mask, and wait for a signal.
- */
-long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize, int p3, int p4,
- int p6, int p7, struct pt_regs *regs)
-{
- sigset_t saveset, newset;
-
- /* XXX: Don't preclude handling different sized sigset_t's. */
- if (sigsetsize != sizeof(sigset_t))
- return -EINVAL;
-
- if (copy_from_user(&newset, unewset, sizeof(newset)))
- return -EFAULT;
- sigdelsetmask(&newset, ~_BLOCKABLE);
-
- spin_lock_irq(¤t->sighand->siglock);
- saveset = current->blocked;
- current->blocked = newset;
- recalc_sigpending();
- spin_unlock_irq(¤t->sighand->siglock);
-
- regs->result = -EINTR;
- regs->gpr[3] = EINTR;
- regs->ccr |= 0x10000000;
- while (1) {
- current->state = TASK_INTERRUPTIBLE;
- schedule();
- if (do_signal(&saveset, regs)) {
- set_thread_flag(TIF_RESTOREALL);
- return 0;
- }
- }
-}
-
long sys_sigaltstack(const stack_t __user *uss, stack_t __user *uoss, unsigned long r5,
unsigned long r6, unsigned long r7, unsigned long r8,
struct pt_regs *regs)
--- a/include/asm-powerpc/unistd.h
+++ b/include/asm-powerpc/unistd.h
@@ -444,11 +446,13 @@ type name(type1 arg1, type2 arg2, type3
#define __ARCH_WANT_SYS_SIGPENDING
#define __ARCH_WANT_SYS_SIGPROCMASK
#define __ARCH_WANT_SYS_RT_SIGACTION
+#define __ARCH_WANT_SYS_RT_SIGSUSPEND
#ifdef CONFIG_PPC32
#define __ARCH_WANT_OLD_STAT
#endif
#ifdef CONFIG_PPC64
#define __ARCH_WANT_COMPAT_SYS_TIME
+#define __ARCH_WANT_COMPAT_SYS_RT_SIGSUSPEND
#endif
/*
--- a/kernel/compat.c
+++ b/kernel/compat.c
@@ -855,3 +855,31 @@ asmlinkage long compat_sys_stime(compat_
}
#endif /* __ARCH_WANT_COMPAT_SYS_TIME */
+
+#ifdef __ARCH_WANT_COMPAT_SYS_RT_SIGSUSPEND
+long compat_sys_rt_sigsuspend(compat_sigset_t __user *unewset, compat_size_t sigsetsize)
+{
+ sigset_t saveset, newset;
+ compat_sigset_t newset32;
+
+ /* XXX: Don't preclude handling different sized sigset_t's. */
+ if (sigsetsize != sizeof(sigset_t))
+ return -EINVAL;
+
+ if (copy_from_user(&newset32, unewset, sizeof(compat_sigset_t)))
+ return -EFAULT;
+ sigset_from_compat(&newset, &newset32);
+ sigdelsetmask(&newset, sigmask(SIGKILL)|sigmask(SIGSTOP));
+
+ spin_lock_irq(¤t->sighand->siglock);
+ saveset = current->blocked;
+ current->blocked = newset;
+ recalc_sigpending();
+ spin_unlock_irq(¤t->sighand->siglock);
+
+ current->state = TASK_INTERRUPTIBLE;
+ schedule();
+ set_thread_flag(TIF_RESTORE_SIGMASK);
+ return -ERESTARTNOHAND;
+}
+#endif /* __ARCH_WANT_COMPAT_SYS_RT_SIGSUSPEND */
diff --git a/kernel/signal.c b/kernel/signal.c
index d7611f1..5e4c240 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2623,6 +2623,32 @@ sys_pause(void)
#endif
+#ifdef __ARCH_WANT_SYS_RT_SIGSUSPEND
+long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize)
+{
+ sigset_t saveset, newset;
+
+ /* XXX: Don't preclude handling different sized sigset_t's. */
+ if (sigsetsize != sizeof(sigset_t))
+ return -EINVAL;
+
+ if (copy_from_user(&newset, unewset, sizeof(newset)))
+ return -EFAULT;
+ sigdelsetmask(&newset, sigmask(SIGKILL)|sigmask(SIGSTOP));
+
+ spin_lock_irq(¤t->sighand->siglock);
+ saveset = current->blocked;
+ current->blocked = newset;
+ recalc_sigpending();
+ spin_unlock_irq(¤t->sighand->siglock);
+
+ current->state = TASK_INTERRUPTIBLE;
+ schedule();
+ set_thread_flag(TIF_RESTORE_SIGMASK);
+ return -ERESTARTNOHAND;
+}
+#endif /* __ARCH_WANT_SYS_RT_SIGSUSPEND */
+
void __init signals_init(void)
{
sigqueue_cachep =
--
dwmw2
David Woodhouse wrote:
>The TIF_RESTORE_SIGMASK flag allows us to have a generic implementation
>of sys_rt_sigsuspend() instead of duplicating it for each architecture.
>This provides such an implementation and makes arch/powerpc use it.
>
>It also tidies up the ppc32 sys_sigsuspend() to use TIF_RESTORE_SIGMASK.
>
>Signed-off-by: David Woodhouse <[email protected]>
>
>
>
>+#ifdef __ARCH_WANT_SYS_RT_SIGSUSPEND
>+long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize)
>+{
>+ sigset_t saveset, newset;
>+
>+ /* XXX: Don't preclude handling different sized sigset_t's. */
>+ if (sigsetsize != sizeof(sigset_t))
>+ return -EINVAL;
>+
>+ if (copy_from_user(&newset, unewset, sizeof(newset)))
>+ return -EFAULT;
>+ sigdelsetmask(&newset, sigmask(SIGKILL)|sigmask(SIGSTOP));
>+
>+ spin_lock_irq(¤t->sighand->siglock);
>+ saveset = current->blocked;
>+ current->blocked = newset;
>+ recalc_sigpending();
>+ spin_unlock_irq(¤t->sighand->siglock);
>+
>+ current->state = TASK_INTERRUPTIBLE;
>+ schedule();
>+ set_thread_flag(TIF_RESTORE_SIGMASK);
>+ return -ERESTARTNOHAND;
>+}
>+#endif /* __ARCH_WANT_SYS_RT_SIGSUSPEND */
>+
>
>
You are not setting saved_sigmask here. And shouldn't it return -EINTR?
Thanks,
Mika
On Tue, 2005-11-29 at 07:18 +0200, Mika Penttilä wrote:
> You are not setting saved_sigmask here.
Oops; well spotted. Thanks.
> And shouldn't it return -EINTR?
I believe not. The previous versions would loop until do_signal()
returned non-zero; i.e. until a signal was actually delivered.
By returning -ERESTARTNOHAND we achieve the same effect. If there's a
signal delivered, that gets magically converted to -EINTR, but if
there's no signal delivered, the syscall gets restarted.
diff -u b/kernel/compat.c b/kernel/compat.c
--- b/kernel/compat.c
+++ b/kernel/compat.c
@@ -859,7 +859,7 @@
#ifdef __ARCH_WANT_COMPAT_SYS_RT_SIGSUSPEND
long compat_sys_rt_sigsuspend(compat_sigset_t __user *unewset, compat_size_t sigsetsize)
{
- sigset_t saveset, newset;
+ sigset_t newset;
compat_sigset_t newset32;
/* XXX: Don't preclude handling different sized sigset_t's. */
@@ -872,7 +872,7 @@
sigdelsetmask(&newset, sigmask(SIGKILL)|sigmask(SIGSTOP));
spin_lock_irq(¤t->sighand->siglock);
- saveset = current->blocked;
+ current->saved_sigmask = current->blocked;
current->blocked = newset;
recalc_sigpending();
spin_unlock_irq(¤t->sighand->siglock);
diff -u b/kernel/signal.c b/kernel/signal.c
--- b/kernel/signal.c
+++ b/kernel/signal.c
@@ -2626,7 +2626,7 @@
#ifdef __ARCH_WANT_SYS_RT_SIGSUSPEND
long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize)
{
- sigset_t saveset, newset;
+ sigset_t newset;
/* XXX: Don't preclude handling different sized sigset_t's. */
if (sigsetsize != sizeof(sigset_t))
@@ -2637,7 +2637,7 @@
sigdelsetmask(&newset, sigmask(SIGKILL)|sigmask(SIGSTOP));
spin_lock_irq(¤t->sighand->siglock);
- saveset = current->blocked;
+ current->saved_sigmask = current->blocked;
current->blocked = newset;
recalc_sigpending();
spin_unlock_irq(¤t->sighand->siglock);
--
dwmw2
On Tue, Nov 29, 2005 at 10:42:03AM +0000, David Woodhouse wrote:
> I believe not. The previous versions would loop until do_signal()
> returned non-zero; i.e. until a signal was actually delivered.
> By returning -ERESTARTNOHAND we achieve the same effect. If there's a
> signal delivered, that gets magically converted to -EINTR, but if
> there's no signal delivered, the syscall gets restarted.
And, crazy coincidence, I think this will fix the recently reported
ptrace attach bug. Right now if you ptrace a process stuck in
sigsuspend, you can't easily force it to return to userspace.
I'll test that if these patches are merged.
--
Daniel Jacobowitz
CodeSourcery, LLC
On Tue, 2005-11-29 at 10:53 -0500, Daniel Jacobowitz wrote:
> And, crazy coincidence, I think this will fix the recently reported
> ptrace attach bug. Right now if you ptrace a process stuck in
> sigsuspend, you can't easily force it to return to userspace.
> I'll test that if these patches are merged.
That seems to be true. What I get with my patches is...
# strace -p `pidof sigsusptest`
Process 1954 attached - interrupt to quit
rt_sigsuspend([]) = ? ERESTARTNOHAND (To be restarted)
--- SIGALRM (Alarm clock) @ 0 (0) ---
sigreturn() = ? (mask now [])
fstat64(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(4, 64), ...}) = 0
ioctl(1, TCGETS, {B115200 opost isig icanon echo ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x3001f000
write(1, "r is -1\n", 8) = 8
munmap(0x3001f000, 4096) = 0
exit_group(8) = ?
Process 1954 detached
... whereas without them I get not only a failure to attach, until
there's a signal, but an unexplained SIGSEGV too...
# strace -p `pidof sigsusptest`
Process 3105 attached - interrupt to quit
--- SIGALRM (Alarm clock) @ 0 (0) ---
rt_sigsuspend([]) = 14
rt_sigsuspend([] <unfinished ...>
--- SIGSEGV (Segmentation fault) @ 0 (0) ---
Process 3105 detached
--
dwmw2