[uml-devel] Signal handling: signal mask incorrect after segfault

[uml-devel] Signal handling: signal mask incorrect after segfault

[Stroesser, Bodo]

A new patch:

If the user stack limit is reached or the signal stack assigned with
sigaltstack() is invalid when a user signal handler with SA_ONSTACK has
to be started, the signal mask of the interrupted user program is
modified. This happens because the mask, that should be used with the
handler only, is written to "current->blocked" even if the handler could
not be started. But without a handler, no rewrite of the original mask
at sys_sigreturn will be done.
A slightly different case is sys_sigsuspend(), where the mask is already
modified when kern_do_signal() is started. "*oldset" and
"current->blocked" are not equal here and thus current->blocked has to
be set to *oldset, if an error occurs in handle_signal().
For both cases I've written small tests, and with the patch the result
is OK.
This issue is relevant for other architectures too (e.g. i386, I've
seen).

The last two hunks are for simplification in do_signal() and
kern_do_signal() only.

Bodo


--- arch/um/kernel/signal_kern.c.orig	2004-09-30 15:13:49.488331887
+0200
+++ arch/um/kernel/signal_kern.c	2004-09-30 16:06:43.918738785
+0200
@@ -88,7 +88,14 @@
 					    (unsigned long)
ka->sa.sa_handler,
 					    restorer, regs, oldset);
 
-	if (!(ka->sa.sa_flags & SA_NODEFER)) {
+	if(err) {
+		spin_lock_irq(&current->sighand->siglock);
+		current->blocked = *oldset;
+		recalc_sigpending();
+		spin_unlock_irq(&current->sighand->siglock);
+		force_sigsegv(signr, current);
+	}
+	else if (!(ka->sa.sa_flags & SA_NODEFER)) {
 		spin_lock_irq(&current->sighand->siglock);
 		sigorsets(&current->blocked, &current->blocked, 
 			  &ka->sa.sa_mask);
@@ -96,9 +103,6 @@
 		recalc_sigpending();
 		spin_unlock_irq(&current->sighand->siglock);
 	}
-
-	if(err)
-		force_sigsegv(signr, current);
 }
 
 static int kern_do_signal(struct pt_regs *regs, sigset_t *oldset)
@@ -107,9 +111,6 @@
 	siginfo_t info;
 	int sig;
 
-	if (!oldset)
-		oldset = &current->blocked;
-
 	sig = get_signal_to_deliver(&info, &ka_copy, regs, NULL);
 	if(sig > 0){
 		/* Whee!  Actually deliver the signal.  */
@@ -147,7 +148,7 @@
 
 int do_signal(void)
 {
-	return(kern_do_signal(&current->thread.regs, NULL));
+	return(kern_do_signal(&current->thread.regs,
&current->blocked));
 }
 
 /*


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] Signal handling: signal mask incorrect after segfault

[Jeff Dike]

Bodo.Stroesser@fujitsu-siemens.com said:
> If the user stack limit is reached or the signal stack assigned with
> sigaltstack() is invalid when a user signal handler with SA_ONSTACK
> has to be started, the signal mask of the interrupted user program is
> modified. This happens because the mask, that should be used with the
> handler only, is written to "current->blocked" even if the handler
> could not be started. But without a handler, no rewrite of the
> original mask at sys_sigreturn will be done.

I don't see where current->blocked is modified up to that point.  It looks 
like this could be fixed by checking err in the !SA_DEFER case:

	if (!err && !(ka->sa.sa_flags & SA_NODEFER)) {

> A slightly different case
> is sys_sigsuspend(), where the mask is already modified when
> kern_do_signal() is started. "*oldset" and "current->blocked" are not
> equal here and thus current->blocked has to be set to *oldset, if an
> error occurs in handle_signal(). For both cases I've written small
> tests, and with the patch the result is OK. 

The existing code looks OK to me.  If there's an error writing the signal
frame, then it will get a SIGSEGV.  If that is caught (and somehow succeeds,
like it's going to a good stack), and it returns, then that's where your
sigreturn and mask restoration is going to happen.

If none of the above happen, then the process is dead meat, of course.

				Jeff



-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

RE: [uml-devel] Signal handling: signal mask incorrect after segfault

[Stroesser, Bodo]

> Bodo.Stroesser@fujitsu-siemens.com said:
> > If the user stack limit is reached or the signal stack assigned with
> > sigaltstack() is invalid when a user signal handler with SA_ONSTACK 
> > has to be started, the signal mask of the interrupted user program
is 
> > modified. This happens because the mask, that should be used with
the 
> > handler only, is written to "current->blocked" even if the handler 
> > could not be started. But without a handler, no rewrite of the 
> > original mask at sys_sigreturn will be done.
> 
> I don't see where current->blocked is modified up to that point.  It
looks like this could be fixed by checking err in the !SA_DEFER case:
> 
> 	if (!err && !(ka->sa.sa_flags & SA_NODEFER)) {

Yes. That is, what the patch does. It uses:

if (err) {
}
else if (!(ka->sa.sa_flags & SA_NODEFER)) {

> > A slightly different case
> > is sys_sigsuspend(), where the mask is already modified when
> > kern_do_signal() is started. "*oldset" and "current->blocked" are
not 
> > equal here and thus current->blocked has to be set to *oldset, if an

> > error occurs in handle_signal(). For both cases I've written small 
> > tests, and with the patch the result is OK.
> 
> The existing code looks OK to me.  If there's an error writing the
signal frame, then it will get a SIGSEGV.  If that is caught (and
somehow succeeds, like it's going to a good stack), and it returns, then
that's where your sigreturn and mask restoration is going to happen.

No. sys_sigsuspend() modifies current->blocked before calling
kern_do_signal(). Because of that, it takes a copy of the original mask
and passes it to kern_do_signal(). This mask has to be restored by the
signal handler at sys_sigreturn. But if an error occurs, the handler
will not be started and the mask will not be restored. Then
kern_do_signal returns to sys_sigsuspend() with the wrong mask set,
sys_sigsuspend() returns (the original mask is lost at that moment) and
the SIGSEGV-handler is started later when passing interrupt_end(), which
calls do_signal() again. The SIGSEGV handler doesn't even see the real
"original mask", because do_signal() now calls kern_do_signal() with
current->blocked as oldset, which is the mask modified by
sys_sigsuspend(). So I believe, handle_signal() should restore the
original mask if setup_signal_stack_XX() returns an error. (For the
first case I described, this is a NOOP.)

On the other hand: with the patch "deliver segfault immediately"
applied, this is redundant, because the "oldset" used to build the next
handler's stack frame will be the same as before, as kern_do_signal()
doesn't return after an error. At least a handler for SIGSEGV will be
started and it will restore the mask, or the process will die.

> 
> If none of the above happen, then the process is dead meat, of course.
> 
> 				Jeff

Bodo


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel