Re: [PATCH] UML - signal bug fix

Re: [PATCH] UML - signal bug fix

[Bodo Stroesser]

Jeff Dike wrote:
> This patch fixes a bug introduced in the last batch of signal fixes.  The
> system call return value should only be reset if called diectly from a system
> call, i.e. sigsuspend.  The fixes added earlier caused any interrupted non-zero
> system call return to be reset, confusing fork and vfork, among others.
> 
> Signed-off-by: Jeff Dike <jdike@addtoit.com>
> 
Yes. The patchset was wrong.
But IMHO the solution shouldn't be resetting to the old state, that did syscall
restarting wrong!
The problem, you fix here, doesn't occur, when using my complete patchset. And my
patches fix UML's wrong syscall restart handling. And other issues they fix, too.

Bodo

[PATCH] UML - signal bug fix

[Jeff Dike]

This patch fixes a bug introduced in the last batch of signal fixes.  The
system call return value should only be reset if called diectly from a system
call, i.e. sigsuspend.  The fixes added earlier caused any interrupted non-zero
system call return to be reset, confusing fork and vfork, among others.

Signed-off-by: Jeff Dike <jdike@addtoit.com>

Index: 2.6.9/arch/um/kernel/signal_kern.c
===================================================================
--- 2.6.9.orig/arch/um/kernel/signal_kern.c	2004-11-08 19:14:45.000000000 -0500
+++ 2.6.9/arch/um/kernel/signal_kern.c	2004-11-11 11:44:05.000000000 -0500
@@ -40,19 +40,19 @@
  */	
 static void handle_signal(struct pt_regs *regs, unsigned long signr,
 			  struct k_sigaction *ka, siginfo_t *info,
-			  sigset_t *oldset)
+			  sigset_t *oldset, int error)
 {
         __sighandler_t handler;
 	void (*restorer)(void);
 	unsigned long sp;
 	sigset_t save;
-	int error, err, ret;
+	int err, ret;
 
-	error = PT_REGS_SYSCALL_RET(&current->thread.regs);
+	err = PT_REGS_SYSCALL_RET(&current->thread.regs);
 	ret = 0;
 	/* Always make any pending restarted system calls return -EINTR */
 	current_thread_info()->restart_block.fn = do_no_restart_syscall;
-	switch(error){
+	switch(err){
 	case -ERESTART_RESTARTBLOCK:
 	case -ERESTARTNOHAND:
 		ret = -EINTR;
@@ -99,7 +99,8 @@
 	if((ka->sa.sa_flags & SA_ONSTACK) && (sas_ss_flags(sp) == 0))
 		sp = current->sas_ss_sp + current->sas_ss_size;
 	
-	if(error != 0) PT_REGS_SET_SYSCALL_RETURN(regs, ret);
+	if(error != 0)
+		PT_REGS_SET_SYSCALL_RETURN(regs, ret);
 
 	if (ka->sa.sa_flags & SA_RESTORER) restorer = ka->sa.sa_restorer;
 	else restorer = NULL;
@@ -114,7 +115,7 @@
 		force_sigsegv(signr, current);
 }
 
-static int kern_do_signal(struct pt_regs *regs, sigset_t *oldset)
+static int kern_do_signal(struct pt_regs *regs, sigset_t *oldset, int error)
 {
 	struct k_sigaction ka_copy;
 	siginfo_t info;
@@ -126,7 +127,7 @@
 	sig = get_signal_to_deliver(&info, &ka_copy, regs, NULL);
 	if(sig > 0){
 		/* Whee!  Actually deliver the signal.  */
-		handle_signal(regs, sig, &ka_copy, &info, oldset);
+		handle_signal(regs, sig, &ka_copy, &info, oldset, error);
 		return(1);
 	}
 
@@ -160,7 +161,7 @@
 
 int do_signal(int error)
 {
-	return(kern_do_signal(&current->thread.regs, NULL));
+	return(kern_do_signal(&current->thread.regs, NULL, error));
 }
 
 /*
@@ -181,7 +182,7 @@
 	while (1) {
 		current->state = TASK_INTERRUPTIBLE;
 		schedule();
-		if(kern_do_signal(&current->thread.regs, &saveset))
+		if(kern_do_signal(&current->thread.regs, &saveset, -EINTR))
 			return(-EINTR);
 	}
 }
@@ -208,7 +209,7 @@
 	while (1) {
 		current->state = TASK_INTERRUPTIBLE;
 		schedule();
-		if (kern_do_signal(&current->thread.regs, &saveset))
+		if (kern_do_signal(&current->thread.regs, &saveset, -EINTR))
 			return(-EINTR);
 	}
 }

Re: [PATCH] UML - signal bug fix

[Jeff Dike]

bstroesser@fujitsu-siemens.com said:
> But IMHO the solution shouldn't be resetting to the old state, that
> did syscall restarting wrong! The problem, you fix here, doesn't
> occur, when using my complete patchset. And my patches fix UML's wrong
> syscall restart handling. And other issues they fix, too. 

OK, I was trying to be minimal with those patches, and I guess I overdid it
a bit.

I'll get the rest of your changes in, and back this fix out in the process.

				Jeff