[PATCH] ARM: prevent multiple syscall restarts

[PATCH] ARM: prevent multiple syscall restarts

[Russell King]

Al Viro reports that calling "sys_sigsuspend(-ERESTARTNOHAND, 0, 0)"
with two signals coming and being handled in kernel space results
in the syscall restart being done twice.

Avoid this by clearing the 'why' flag when we call the signal handling
code to prevent further syscall restarts after the first.

Acked-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/kernel/entry-common.S |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index f05a35a..4a560d3 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -48,6 +48,8 @@ work_pending:
 	beq	no_work_pending
 	mov	r0, sp				@ 'regs'
 	mov	r2, why				@ 'syscall'
+	tst	r1, #_TIF_SIGPENDING		@ delivering a signal?
+	movne	why, #0				@ prevent further restarts
 	bl	do_notify_resume
 	b	ret_slow_syscall		@ Check work again
 
-- 
1.6.2.5


-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH] ARM: prevent multiple syscall restarts

[Al Viro]

On Fri, Sep 17, 2010 at 04:21:58PM +0100, Russell King wrote:
> Al Viro reports that calling "sys_sigsuspend(-ERESTARTNOHAND, 0, 0)"
> with two signals coming and being handled in kernel space results
> in the syscall restart being done twice.
> 
> Avoid this by clearing the 'why' flag when we call the signal handling
> code to prevent further syscall restarts after the first.

I'd like to add that while sigsuspend example is, certainly, cheating
(the first two arguments of sigsuspend(2) on arm are ignored for
historical reasons, and libc is not going to end up with that in
the first argument), the issue is real - existing code on arm relied
on the following assertion:

	no syscall called with -ERESTART... in the first argument will
ever return -ERESTART...

It happens to be true for most of the syscalls, since that value will lead
to -EBADF if the argument is a file descriptor and -EFAULT if it's a userland
pointer.  Ones that expect a syscall-specific enum will probably also just
fail with -EINVAL.  So most of the potential victims are actually OK.  However,
it's certainly better to have a straightforward prevention of that kind of
crap than keep relying on fairly subtle properties of code in core kernel
that has no idea about arch/arm relying on it - in undocumented way, at that.