2006-01-10 20:19:25

by David Woodhouse

[permalink] [raw]
Subject: [PATCH] [5/6] Handle TIF_RESTORE_SIGMASK for i386

The attached patch handles TIF_RESTORE_SIGMASK as added by David Woodhouse's
patch entitled:

[PATCH] 2/3 Add TIF_RESTORE_SIGMASK support for arch/powerpc
[PATCH] 3/3 Generic sys_rt_sigsuspend

It does the following:

(1) Declares TIF_RESTORE_SIGMASK for i386.

(2) Invokes it over to do_signal() when TIF_RESTORE_SIGMASK is set.

(3) Makes do_signal() support TIF_RESTORE_SIGMASK, using the signal mask saved
in current->saved_sigmask.

(4) Discards sys_rt_sigsuspend() from the arch, using the generic one instead.

(5) Makes sys_sigsuspend() save the signal mask and set TIF_RESTORE_SIGMASK
rather than attempting to fudge the return registers.

(6) Makes sys_sigsuspend() return -ERESTARTNOHAND rather than looping
intrinsically.

(7) Makes setup_frame(), setup_rt_frame() and handle_signal() return 0 or
-EFAULT rather than true/false to be consistent with the rest of the
kernel.

Due to the fact do_signal() is then only called from one place:

(8) Makes do_signal() no longer have a return value is it was just being
ignored; force_sig() takes care of this.

(9) Discards the old sigmask argument to do_signal() as it's no longer
necessary.

(10) Makes do_signal() static.

(11) Marks the second argument to do_notify_resume() as unused. The unused
argument should remain in the middle as the arguments are passed in as
registers, and the ordering is specific in entry.S

Given the way do_signal() is now no longer called from sys_{,rt_}sigsuspend(),
they no longer need access to the exception frame, and so can just take
arguments normally.

This patch depends on sys_rt_sigsuspend patch.

Signed-off-by: David Howells <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>

arch/i386/kernel/signal.c | 109 ++++++++++++++++++-----------------------
include/asm-i386/signal.h | 1
include/asm-i386/thread_info.h | 2
include/asm-i386/unistd.h | 1
4 files changed, 51 insertions(+), 62 deletions(-)

diff --git a/arch/i386/kernel/signal.c b/arch/i386/kernel/signal.c
index adcd069..963616d 100644
--- a/arch/i386/kernel/signal.c
+++ b/arch/i386/kernel/signal.c
@@ -37,51 +37,17 @@
asmlinkage int
sys_sigsuspend(int history0, int history1, old_sigset_t mask)
{
- struct pt_regs * regs = (struct pt_regs *) &history0;
- sigset_t saveset;
-
mask &= _BLOCKABLE;
spin_lock_irq(&current->sighand->siglock);
- saveset = current->blocked;
+ current->saved_sigmask = current->blocked;
siginitset(&current->blocked, mask);
recalc_sigpending();
spin_unlock_irq(&current->sighand->siglock);

- regs->eax = -EINTR;
- while (1) {
- current->state = TASK_INTERRUPTIBLE;
- schedule();
- if (do_signal(regs, &saveset))
- return -EINTR;
- }
-}
-
-asmlinkage int
-sys_rt_sigsuspend(struct pt_regs regs)
-{
- sigset_t saveset, newset;
-
- /* XXX: Don't preclude handling different sized sigset_t's. */
- if (regs.ecx != sizeof(sigset_t))
- return -EINVAL;
-
- if (copy_from_user(&newset, (sigset_t __user *)regs.ebx, sizeof(newset)))
- return -EFAULT;
- sigdelsetmask(&newset, ~_BLOCKABLE);
-
- spin_lock_irq(&current->sighand->siglock);
- saveset = current->blocked;
- current->blocked = newset;
- recalc_sigpending();
- spin_unlock_irq(&current->sighand->siglock);
-
- regs.eax = -EINTR;
- while (1) {
- current->state = TASK_INTERRUPTIBLE;
- schedule();
- if (do_signal(&regs, &saveset))
- return -EINTR;
- }
+ current->state = TASK_INTERRUPTIBLE;
+ schedule();
+ set_thread_flag(TIF_RESTORE_SIGMASK);
+ return -ERESTARTNOHAND;
}

asmlinkage int
@@ -433,11 +399,11 @@ static int setup_frame(int sig, struct k
current->comm, current->pid, frame, regs->eip, frame->pretcode);
#endif

- return 1;
+ return 0;

give_sigsegv:
force_sigsegv(sig, current);
- return 0;
+ return -EFAULT;
}

static int setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
@@ -527,11 +493,11 @@ static int setup_rt_frame(int sig, struc
current->comm, current->pid, frame, regs->eip, frame->pretcode);
#endif

- return 1;
+ return 0;

give_sigsegv:
force_sigsegv(sig, current);
- return 0;
+ return -EFAULT;
}

/*
@@ -581,7 +547,7 @@ handle_signal(unsigned long sig, siginfo
else
ret = setup_frame(sig, ka, oldset, regs);

- if (ret) {
+ if (ret == 0) {
spin_lock_irq(&current->sighand->siglock);
sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
if (!(ka->sa.sa_flags & SA_NODEFER))
@@ -598,11 +564,12 @@ handle_signal(unsigned long sig, siginfo
* want to handle. Thus you cannot kill init even with a SIGKILL even by
* mistake.
*/
-int fastcall do_signal(struct pt_regs *regs, sigset_t *oldset)
+static void fastcall do_signal(struct pt_regs *regs)
{
siginfo_t info;
int signr;
struct k_sigaction ka;
+ sigset_t *oldset;

/*
* We want the common case to go fast, which
@@ -613,12 +580,14 @@ int fastcall do_signal(struct pt_regs *r
* CS suffices.
*/
if (!user_mode(regs))
- return 1;
+ return;

if (try_to_freeze())
goto no_signal;

- if (!oldset)
+ if (test_thread_flag(TIF_RESTORE_SIGMASK))
+ oldset = &current->saved_sigmask;
+ else
oldset = &current->blocked;

signr = get_signal_to_deliver(&info, &ka, regs, NULL);
@@ -628,38 +597,55 @@ int fastcall do_signal(struct pt_regs *r
* have been cleared if the watchpoint triggered
* inside the kernel.
*/
- if (unlikely(current->thread.debugreg[7])) {
+ if (unlikely(current->thread.debugreg[7]))
set_debugreg(current->thread.debugreg[7], 7);
- }

/* Whee! Actually deliver the signal. */
- return handle_signal(signr, &info, &ka, oldset, regs);
+ if (handle_signal(signr, &info, &ka, oldset, regs) == 0) {
+ /* a signal was successfully delivered; the saved
+ * sigmask will have been stored in the signal frame,
+ * and will be restored by sigreturn, so we can simply
+ * clear the TIF_RESTORE_SIGMASK flag */
+ if (test_thread_flag(TIF_RESTORE_SIGMASK))
+ clear_thread_flag(TIF_RESTORE_SIGMASK);
+ }
+
+ return;
}

- no_signal:
+no_signal:
/* Did we come from a system call? */
if (regs->orig_eax >= 0) {
/* Restart the system call - no handlers present */
- if (regs->eax == -ERESTARTNOHAND ||
- regs->eax == -ERESTARTSYS ||
- regs->eax == -ERESTARTNOINTR) {
+ switch (regs->eax) {
+ case -ERESTARTNOHAND:
+ case -ERESTARTSYS:
+ case -ERESTARTNOINTR:
regs->eax = regs->orig_eax;
regs->eip -= 2;
- }
- if (regs->eax == -ERESTART_RESTARTBLOCK){
+ break;
+
+ case -ERESTART_RESTARTBLOCK:
regs->eax = __NR_restart_syscall;
regs->eip -= 2;
+ break;
}
}
- return 0;
+
+ /* if there's no signal to deliver, we just put the saved sigmask
+ * back */
+ if (test_thread_flag(TIF_RESTORE_SIGMASK)) {
+ clear_thread_flag(TIF_RESTORE_SIGMASK);
+ sigprocmask(SIG_SETMASK, &current->saved_sigmask, NULL);
+ }
}

/*
* notification of userspace execution resumption
- * - triggered by current->work.notify_resume
+ * - triggered by the TIF_WORK_MASK flags
*/
__attribute__((regparm(3)))
-void do_notify_resume(struct pt_regs *regs, sigset_t *oldset,
+void do_notify_resume(struct pt_regs *regs, void *_unused,
__u32 thread_info_flags)
{
/* Pending single-step? */
@@ -667,9 +653,10 @@ void do_notify_resume(struct pt_regs *re
regs->eflags |= TF_MASK;
clear_thread_flag(TIF_SINGLESTEP);
}
+
/* deal with pending signal delivery */
- if (thread_info_flags & _TIF_SIGPENDING)
- do_signal(regs,oldset);
+ if (thread_info_flags & (_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK))
+ do_signal(regs);

clear_thread_flag(TIF_IRET);
}
diff --git a/include/asm-i386/signal.h b/include/asm-i386/signal.h
index 76524b4..026fd23 100644
--- a/include/asm-i386/signal.h
+++ b/include/asm-i386/signal.h
@@ -218,7 +218,6 @@ static __inline__ int sigfindinword(unsi
}

struct pt_regs;
-extern int FASTCALL(do_signal(struct pt_regs *regs, sigset_t *oldset));

#define ptrace_signal_deliver(regs, cookie) \
do { \
diff --git a/include/asm-i386/thread_info.h b/include/asm-i386/thread_info.h
index 8fbf791..d080eea 100644
--- a/include/asm-i386/thread_info.h
+++ b/include/asm-i386/thread_info.h
@@ -142,6 +142,7 @@ register unsigned long current_stack_poi
#define TIF_SYSCALL_EMU 6 /* syscall emulation active */
#define TIF_SYSCALL_AUDIT 7 /* syscall auditing active */
#define TIF_SECCOMP 8 /* secure computing */
+#define TIF_RESTORE_SIGMASK 9 /* restore signal mask in do_signal() */
#define TIF_POLLING_NRFLAG 16 /* true if poll_idle() is polling TIF_NEED_RESCHED */
#define TIF_MEMDIE 17

@@ -154,6 +155,7 @@ register unsigned long current_stack_poi
#define _TIF_SYSCALL_EMU (1<<TIF_SYSCALL_EMU)
#define _TIF_SYSCALL_AUDIT (1<<TIF_SYSCALL_AUDIT)
#define _TIF_SECCOMP (1<<TIF_SECCOMP)
+#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK)
#define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG)

/* work to do on interrupt/exception return */
diff --git a/include/asm-i386/unistd.h b/include/asm-i386/unistd.h
index 481c3c0..4a8d5ae 100644
--- a/include/asm-i386/unistd.h
+++ b/include/asm-i386/unistd.h
@@ -417,6 +417,7 @@ __syscall_return(type,__res); \
#define __ARCH_WANT_SYS_SIGPENDING
#define __ARCH_WANT_SYS_SIGPROCMASK
#define __ARCH_WANT_SYS_RT_SIGACTION
+#define __ARCH_WANT_SYS_RT_SIGSUSPEND
#endif

#ifdef __KERNEL_SYSCALLS__

--
dwmw2


2006-01-13 04:00:26

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] [5/6] Handle TIF_RESTORE_SIGMASK for i386

David Woodhouse <[email protected]> wrote:
>
> The attached patch handles TIF_RESTORE_SIGMASK as added by David Woodhouse's
> patch entitled:
>
> [PATCH] 2/3 Add TIF_RESTORE_SIGMASK support for arch/powerpc
> [PATCH] 3/3 Generic sys_rt_sigsuspend
>
> It does the following:
>
> (1) Declares TIF_RESTORE_SIGMASK for i386.
>
> (2) Invokes it over to do_signal() when TIF_RESTORE_SIGMASK is set.
>
> (3) Makes do_signal() support TIF_RESTORE_SIGMASK, using the signal mask saved
> in current->saved_sigmask.
>
> (4) Discards sys_rt_sigsuspend() from the arch, using the generic one instead.
>
> (5) Makes sys_sigsuspend() save the signal mask and set TIF_RESTORE_SIGMASK
> rather than attempting to fudge the return registers.
>
> (6) Makes sys_sigsuspend() return -ERESTARTNOHAND rather than looping
> intrinsically.
>
> (7) Makes setup_frame(), setup_rt_frame() and handle_signal() return 0 or
> -EFAULT rather than true/false to be consistent with the rest of the
> kernel.
>
> Due to the fact do_signal() is then only called from one place:
>
> (8) Makes do_signal() no longer have a return value is it was just being
> ignored; force_sig() takes care of this.
>
> (9) Discards the old sigmask argument to do_signal() as it's no longer
> necessary.
>
> (10) Makes do_signal() static.
>
> (11) Marks the second argument to do_notify_resume() as unused. The unused
> argument should remain in the middle as the arguments are passed in as
> registers, and the ordering is specific in entry.S
>
> Given the way do_signal() is now no longer called from sys_{,rt_}sigsuspend(),
> they no longer need access to the exception frame, and so can just take
> arguments normally.
>
> This patch depends on sys_rt_sigsuspend patch.

I have problems with this patch.

With

generic-sys_rt_sigsuspend.patch
handle-tif_restore_sigmask-for-frv.patch
handle-tif_restore_sigmask-for-i386.patch

applied, or with all of David's patches applied, an FC5-test1 machine hangs
during the login process (local vt or sshd). An FC1 machine doesn't
exhibit the problem.

dmesg+sysrq-t:
http://www.zip.com.au/~akpm/linux/patches/stuff/dmesg

.config:
http://www.zip.com.au/~akpm/linux/patches/stuff/config-sony

culprit patches:
http://www.zip.com.au/~akpm/linux/patches/stuff/generic-sys_rt_sigsuspend.patch
http://www.zip.com.au/~akpm/linux/patches/stuff/handle-tif_restore_sigmask-for-i386.patch

(I retested with just current -linus and the above two patches. Same
deal).

2006-01-13 04:30:06

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] [5/6] Handle TIF_RESTORE_SIGMASK for i386

On Thu, 2006-01-12 at 19:59 -0800, Andrew Morton wrote:
> applied, or with all of David's patches applied, an FC5-test1 machine hangs
> during the login process (local vt or sshd). An FC1 machine doesn't
> exhibit the problem.

So the 'successful' login reported at 19:21:45 never actually completed
and gave you a shell?

I can't make out which process it is that's misbehaving... and your
login was pid 2292 but I don't see your SysRq-T output going up that
far. Am I missing something?

I note you're running auditd -- FC5-test1 enabled syscall auditing by
default. Does the problem persist if you prevent the auditd initscript
from starting up? If so, let's turn auditing back on and actually make
use of it -- assuming the offending process is actually one of your own
after the login has changed uid, can you set an audit rule to log all
syscalls from your own userid? (add '-Aexit,always -Fuid=500'
to /etc/audit.rules, assuming 500 is your own uid). Then show me the
appropriate section from /var/log/audit/audit.log.

I tested both with and without audit on PPC -- David, did you test this
patch with auditing enabled on i386?

Will attempt to reproduce locally... I've _also_ seen login hangs on
current linus trees but they've been different (and on that machine I
haven't had the TIF_RESTORE_SIGMASK patches either). It happens on disk
activity though -- after 'rpm -i <kernelpackage>' the whole machine
locks up and I have no more file system access. If your SysRq-T got to
the disk, I suspect you aren't seeing the same problem.

--
dwmw2

2006-01-13 04:55:30

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] [5/6] Handle TIF_RESTORE_SIGMASK for i386

David Woodhouse <[email protected]> wrote:
>
> On Thu, 2006-01-12 at 19:59 -0800, Andrew Morton wrote:
> > applied, or with all of David's patches applied, an FC5-test1 machine hangs
> > during the login process (local vt or sshd). An FC1 machine doesn't
> > exhibit the problem.
>
> So the 'successful' login reported at 19:21:45 never actually completed
> and gave you a shell?

Yup.

> I can't make out which process it is that's misbehaving... and your
> login was pid 2292 but I don't see your SysRq-T output going up that
> far. Am I missing something?

Yes, that dmesg output seems to have been truncated.

Here's another one, better:

http://www.zip.com.au/~akpm/linux/patches/stuff/dmesg

Note that I have /bin/zsh in /etc/passwd.

> I note you're running auditd -- FC5-test1 enabled syscall auditing by
> default.

This is basically the fc5-t1 .config, only it has selinux turned off due to
earlier problems. Suggest you base testing on my config-sony.

> Does the problem persist if you prevent the auditd initscript
> from starting up?

<chkconfig auditd off>
<reboot>
<problem persists>

> If so, let's turn auditing back on

<chkconfig auditd on>

> and actually make
> use of it -- assuming the offending process is actually one of your own
> after the login has changed uid, can you set an audit rule to log all
> syscalls from your own userid? (add '-Aexit,always -Fuid=500'
> to /etc/audit.rules, assuming 500 is your own uid). Then show me the
> appropriate section from /var/log/audit/audit.log.

<does that>
<reboots>
<logs in>

You wouldn't believe how much stuff that produces. Or maybe you would.

<several minutes pass, disk LED flashing>

<crap starts scrolling past too fast to read. Some complaint from auditd, afaict>

<does alt-SUB>

<grabs the last bit of auditd.log>

http://www.zip.com.au/~akpm/linux/patches/stuff/auditd.log

That looks like the crap I saw scrolling past. How come it came out on the
console after a few minutes?

> I tested both with and without audit on PPC -- David, did you test this
> patch with auditing enabled on i386?
>
> Will attempt to reproduce locally... I've _also_ seen login hangs on
> current linus trees but they've been different (and on that machine I
> haven't had the TIF_RESTORE_SIGMASK patches either). It happens on disk
> activity though -- after 'rpm -i <kernelpackage>' the whole machine
> locks up and I have no more file system access. If your SysRq-T got to
> the disk, I suspect you aren't seeing the same problem.

Sounds like the jens-barrier-bug. Fixed in current -linus.

2006-01-13 05:15:40

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] [5/6] Handle TIF_RESTORE_SIGMASK for i386

On Thu, 2006-01-12 at 20:54 -0800, Andrew Morton wrote:
> Yes, that dmesg output seems to have been truncated.
>
> Here's another one, better:
>
> http://www.zip.com.au/~akpm/linux/patches/stuff/dmesg
>
> Note that I have /bin/zsh in /etc/passwd.

Indeed better; thanks. Will ponder and attempt to reproduce here on
i386.

> That looks like the crap I saw scrolling past. How come it came out on the
> console after a few minutes?

Not sure. That probably means the kernel has decided that your userspace
audit dæmon has gone AWOL.... and indeed it seems to be stuck on a
futex. I know of no reason why it should be doing that -- Steve?

--
dwmw2

2006-01-13 06:11:08

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] [5/6] Handle TIF_RESTORE_SIGMASK for i386

Andrew Morton <[email protected]> wrote:
>
> Note that I have /bin/zsh in /etc/passwd.
>

It's zsh. I can log in as root (root uses bash) and just type "zsh" and
zsh gets stuck chewing 100% CPU. strace says:


rt_sigprocmask(SIG_BLOCK, [CHLD], [CHLD], 8) = 0
rt_sigsuspend(~[HUP CHLD RTMIN RT_1]) = -1 EINVAL (Invalid argument)
rt_sigprocmask(SIG_BLOCK, [CHLD], [CHLD], 8) = 0
rt_sigsuspend(~[HUP CHLD RTMIN RT_1]) = -1 EINVAL (Invalid argument)
rt_sigprocmask(SIG_BLOCK, [CHLD], [CHLD], 8) = 0
rt_sigsuspend(~[HUP CHLD RTMIN RT_1]) = -1 EINVAL (Invalid argument)
rt_sigprocmask(SIG_BLOCK, [CHLD], [CHLD], 8) = 0
rt_sigsuspend(~[HUP CHLD RTMIN RT_1]) = -1 EINVAL (Invalid argument)
rt_sigprocmask(SIG_BLOCK, [CHLD], [CHLD], 8) = 0
rt_sigsuspend(~[HUP CHLD RTMIN RT_1]) = -1 EINVAL (Invalid argument)
rt_sigprocmask(SIG_BLOCK, [CHLD], [CHLD], 8) = 0
rt_sigsuspend(~[HUP CHLD RTMIN RT_1]) = -1 EINVAL (Invalid argument)
rt_sigprocmask(SIG_BLOCK, [CHLD], [CHLD], 8) = 0
rt_sigsuspend(~[HUP CHLD RTMIN RT_1]) = -1 EINVAL (Invalid argument)
rt_sigprocmask(SIG_BLOCK, [CHLD], [CHLD], 8) = 0
rt_sigsuspend(~[HUP CHLD RTMIN RT_1]) = -1 EINVAL (Invalid argument)

2006-01-13 06:23:32

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] [5/6] Handle TIF_RESTORE_SIGMASK for i386

On Thu, 2006-01-12 at 22:10 -0800, Andrew Morton wrote:
> rt_sigprocmask(SIG_BLOCK, [CHLD], [CHLD], 8) = 0
> rt_sigsuspend(~[HUP CHLD RTMIN RT_1]) = -1 EINVAL (Invalid argument)

OK, that makes it easier to track down -- thanks. Unfortunately my
scratch i386 box in the office hasn't rebooted onto my test kernel so
it'll have to wait until (a sensible hour in) the morning.

I suspect it might be the sigset size. We have a whole bunch of

/* XXX: Don't preclude handling different sized sigset_t's. */
if (sigsetsize != sizeof(sigset_t))
return -EINVAL;

--
dwmw2

2006-01-13 08:28:50

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] [5/6] Handle TIF_RESTORE_SIGMASK for i386

On Fri, 2006-01-13 at 06:23 +0000, David Woodhouse wrote:
> I suspect it might be the sigset size.

I bet this is it, although I haven't yet confirmed the theory....

Sorry, I should have tested this for myself on i386 before forwarding
the patch. Stupid bloody legacy architecture :)

--- linux-2.6.15.ppc/kernel/compat.c~ 2006-01-13 04:39:54.000000000 +0000
+++ linux-2.6.15.ppc/kernel/compat.c 2006-01-13 08:23:24.000000000 +0000
@@ -873,7 +873,7 @@ 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)
+asmlinkage long compat_sys_rt_sigsuspend(compat_sigset_t __user *unewset, compat_size_t sigsetsize)
{
sigset_t newset;
compat_sigset_t newset32;
--- linux-2.6.15.ppc/kernel/signal.c~ 2006-01-13 04:39:54.000000000 +0000
+++ linux-2.6.15.ppc/kernel/signal.c 2006-01-13 08:23:29.000000000 +0000
@@ -2761,7 +2761,7 @@ sys_pause(void)
#endif

#ifdef __ARCH_WANT_SYS_RT_SIGSUSPEND
-long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize)
+asmlinkage long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize)
{
sigset_t newset;


--
dwmw2

2006-01-13 08:49:21

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] [5/6] Handle TIF_RESTORE_SIGMASK for i386

David Woodhouse <[email protected]> wrote:
>
> On Fri, 2006-01-13 at 06:23 +0000, David Woodhouse wrote:
> > I suspect it might be the sigset size.
>
> I bet this is it, although I haven't yet confirmed the theory....
>
> Sorry, I should have tested this for myself on i386 before forwarding
> the patch. Stupid bloody legacy architecture :)
>
> --- linux-2.6.15.ppc/kernel/compat.c~ 2006-01-13 04:39:54.000000000 +0000
> +++ linux-2.6.15.ppc/kernel/compat.c 2006-01-13 08:23:24.000000000 +0000
> @@ -873,7 +873,7 @@ 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)
> +asmlinkage long compat_sys_rt_sigsuspend(compat_sigset_t __user *unewset, compat_size_t sigsetsize)
> {
> sigset_t newset;
> compat_sigset_t newset32;
> --- linux-2.6.15.ppc/kernel/signal.c~ 2006-01-13 04:39:54.000000000 +0000
> +++ linux-2.6.15.ppc/kernel/signal.c 2006-01-13 08:23:29.000000000 +0000
> @@ -2761,7 +2761,7 @@ sys_pause(void)
> #endif
>
> #ifdef __ARCH_WANT_SYS_RT_SIGSUSPEND
> -long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize)
> +asmlinkage long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize)
> {
> sigset_t newset;

I gues we want that, but x86 doesn't compile kernel/compat.c

2006-01-13 08:51:35

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] [5/6] Handle TIF_RESTORE_SIGMASK for i386

Andrew Morton <[email protected]> wrote:
>
> but x86 doesn't compile kernel/compat.c

But it usually compiles singal.c ;) Hang on a mo..

2006-01-13 09:19:16

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] [5/6] Handle TIF_RESTORE_SIGMASK for i386

David Woodhouse <[email protected]> wrote:
>
> -long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize)
> +asmlinkage long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize)

Fixed, thanks.

2006-01-15 17:01:26

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [PATCH] [5/6] Handle TIF_RESTORE_SIGMASK for i386

David Woodhouse wrote:
>
> The attached patch handles TIF_RESTORE_SIGMASK as added by David Woodhouse's
> patch entitled:
>
> [PATCH] 2/3 Add TIF_RESTORE_SIGMASK support for arch/powerpc
> [PATCH] 3/3 Generic sys_rt_sigsuspend
>
> It does the following:
>
> (1) Declares TIF_RESTORE_SIGMASK for i386.
>
> (2) Invokes it over to do_signal() when TIF_RESTORE_SIGMASK is set.
>
> (3) Makes do_signal() support TIF_RESTORE_SIGMASK, using the signal mask saved
> in current->saved_sigmask.

Imho, there is a slightly better way to do it, see the patch below.

Instead of adding TIF_RESTORE_SIGMASK we can introduce ERESTART_SIGBLOCK
return value. Now,

sys_rt_sigsuspend(sigset_t *newset)
{
current->saved_sigmask = current->blocked;
current->blocked = newset;

current->state = TASK_INTERRUPTIBLE;
schedule();
return -ERESTART_SIGBLOCK;
}

sys_pselect()
{
...

ret = core_sys_select();

if (ret == -ERESTARTNOHAND)
ret = -ERESTART_SIGBLOCK;
}

handle_signal()
{
sigset_t *oldset; // removed from param list

...

oldset = &current->blocked;
if (regs->eax == -ERESTART_SIGBLOCK)
oldset = &current->saved_sigmask;

...
}

Comments?

Only for illustration, but seems to work.

include/linux/errno.h | 1
include/linux/sched.h | 2 -
include/asm-i386/signal.h | 3 -
arch/i386/kernel/signal.c | 73 +++++++++++++++++++++-------------------------
4 files changed, 36 insertions(+), 43 deletions(-)

--- 2.6.15/include/linux/errno.h~1_SIGM 2004-09-13 09:33:11.000000000 +0400
+++ 2.6.15/include/linux/errno.h 2006-01-15 18:42:17.000000000 +0300
@@ -11,6 +11,7 @@
#define ERESTARTNOHAND 514 /* restart if no handler.. */
#define ENOIOCTLCMD 515 /* No ioctl command */
#define ERESTART_RESTARTBLOCK 516 /* restart by calling sys_restart_syscall */
+#define ERESTART_SIGBLOCK 517

/* Defined for the NFSv3 protocol */
#define EBADHANDLE 521 /* Illegal NFS file handle */
--- 2.6.15/include/linux/sched.h~1_SIGM 2005-12-06 23:33:16.000000000 +0300
+++ 2.6.15/include/linux/sched.h 2006-01-15 18:55:27.000000000 +0300
@@ -799,7 +799,7 @@ struct task_struct {
struct signal_struct *signal;
struct sighand_struct *sighand;

- sigset_t blocked, real_blocked;
+ sigset_t blocked, real_blocked, saved_blocked;
struct sigpending pending;

unsigned long sas_ss_sp;
--- 2.6.15/include/asm-i386/signal.h~1_SIGM 2005-11-22 19:35:52.000000000 +0300
+++ 2.6.15/include/asm-i386/signal.h 2006-01-15 19:26:58.000000000 +0300
@@ -217,9 +217,6 @@ static __inline__ int sigfindinword(unsi
return word;
}

-struct pt_regs;
-extern int FASTCALL(do_signal(struct pt_regs *regs, sigset_t *oldset));
-
#define ptrace_signal_deliver(regs, cookie) \
do { \
if (current->ptrace & PT_DTRACE) { \
--- 2.6.15/arch/i386/kernel/signal.c~1_SIGM 2005-10-11 16:22:42.000000000 +0400
+++ 2.6.15/arch/i386/kernel/signal.c 2006-01-15 23:13:36.000000000 +0300
@@ -37,29 +37,22 @@
asmlinkage int
sys_sigsuspend(int history0, int history1, old_sigset_t mask)
{
- struct pt_regs * regs = (struct pt_regs *) &history0;
- sigset_t saveset;
-
mask &= _BLOCKABLE;
spin_lock_irq(&current->sighand->siglock);
- saveset = current->blocked;
+ current->saved_blocked = current->blocked;
siginitset(&current->blocked, mask);
recalc_sigpending();
spin_unlock_irq(&current->sighand->siglock);

- regs->eax = -EINTR;
- while (1) {
- current->state = TASK_INTERRUPTIBLE;
- schedule();
- if (do_signal(regs, &saveset))
- return -EINTR;
- }
+ current->state = TASK_INTERRUPTIBLE;
+ schedule();
+ return -ERESTART_SIGBLOCK;
}

asmlinkage int
sys_rt_sigsuspend(struct pt_regs regs)
{
- sigset_t saveset, newset;
+ sigset_t newset;

/* XXX: Don't preclude handling different sized sigset_t's. */
if (regs.ecx != sizeof(sigset_t))
@@ -69,19 +62,11 @@ sys_rt_sigsuspend(struct pt_regs regs)
return -EFAULT;
sigdelsetmask(&newset, ~_BLOCKABLE);

- spin_lock_irq(&current->sighand->siglock);
- saveset = current->blocked;
- current->blocked = newset;
- recalc_sigpending();
- spin_unlock_irq(&current->sighand->siglock);
+ sigprocmask(SIG_SETMASK, &newset, &current->saved_blocked);

- regs.eax = -EINTR;
- while (1) {
- current->state = TASK_INTERRUPTIBLE;
- schedule();
- if (do_signal(&regs, &saveset))
- return -EINTR;
- }
+ current->state = TASK_INTERRUPTIBLE;
+ schedule();
+ return -ERESTART_SIGBLOCK;
}

asmlinkage int
@@ -540,14 +525,18 @@ give_sigsegv:

static int
handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
- sigset_t *oldset, struct pt_regs * regs)
+ struct pt_regs * regs)
{
+ sigset_t *oldset;
+ int saved_blocked = 0;
int ret;

/* Are we from a system call? */
if (regs->orig_eax >= 0) {
/* If so, check system call restarting.. */
switch (regs->eax) {
+ case -ERESTART_SIGBLOCK:
+ saved_blocked = 1;
case -ERESTART_RESTARTBLOCK:
case -ERESTARTNOHAND:
regs->eax = -EINTR;
@@ -575,6 +564,10 @@ handle_signal(unsigned long sig, siginfo
regs->eflags &= ~TF_MASK;
}

+ oldset = &current->blocked;
+ if (saved_blocked)
+ oldset = &current->saved_blocked;
+
/* Set up the stack frame */
if (ka->sa.sa_flags & SA_SIGINFO)
ret = setup_rt_frame(sig, ka, info, oldset, regs);
@@ -588,7 +581,8 @@ handle_signal(unsigned long sig, siginfo
sigaddset(&current->blocked,sig);
recalc_sigpending();
spin_unlock_irq(&current->sighand->siglock);
- }
+ } else if (saved_blocked)
+ regs->eax = -ERESTART_SIGBLOCK;

return ret;
}
@@ -598,7 +592,7 @@ handle_signal(unsigned long sig, siginfo
* want to handle. Thus you cannot kill init even with a SIGKILL even by
* mistake.
*/
-int fastcall do_signal(struct pt_regs *regs, sigset_t *oldset)
+static void fastcall do_signal(struct pt_regs *regs)
{
siginfo_t info;
int signr;
@@ -613,14 +607,11 @@ int fastcall do_signal(struct pt_regs *r
* CS suffices.
*/
if (!user_mode(regs))
- return 1;
+ return;

if (try_to_freeze())
goto no_signal;

- if (!oldset)
- oldset = &current->blocked;
-
signr = get_signal_to_deliver(&info, &ka, regs, NULL);
if (signr > 0) {
/* Reenable any watchpoints before delivering the
@@ -633,25 +624,29 @@ int fastcall do_signal(struct pt_regs *r
}

/* Whee! Actually deliver the signal. */
- return handle_signal(signr, &info, &ka, oldset, regs);
+ handle_signal(signr, &info, &ka, regs);
+ return;
}

no_signal:
/* Did we come from a system call? */
if (regs->orig_eax >= 0) {
/* Restart the system call - no handlers present */
- if (regs->eax == -ERESTARTNOHAND ||
- regs->eax == -ERESTARTSYS ||
- regs->eax == -ERESTARTNOINTR) {
+ switch (regs->eax) {
+ case -ERESTART_SIGBLOCK:
+ sigprocmask(SIG_SETMASK, &current->saved_blocked, NULL);
+ case -ERESTARTSYS:
+ case -ERESTARTNOHAND:
+ case -ERESTARTNOINTR:
regs->eax = regs->orig_eax;
regs->eip -= 2;
- }
- if (regs->eax == -ERESTART_RESTARTBLOCK){
+ break;
+
+ case -ERESTART_RESTARTBLOCK:
regs->eax = __NR_restart_syscall;
regs->eip -= 2;
}
}
- return 0;
}

/*
@@ -669,7 +664,7 @@ void do_notify_resume(struct pt_regs *re
}
/* deal with pending signal delivery */
if (thread_info_flags & _TIF_SIGPENDING)
- do_signal(regs,oldset);
+ do_signal(regs);

clear_thread_flag(TIF_IRET);
}