All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Metcalf <cmetcalf@tilera.com>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>,
	linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: situation with signals
Date: Wed, 27 Oct 2010 18:51:12 -0400	[thread overview]
Message-ID: <4CC8ACE0.1040407@tilera.com> (raw)
In-Reply-To: <20101027213745.GI19804@ZenIV.linux.org.uk>

On 10/27/2010 5:37 PM, Al Viro wrote:
> On Wed, Oct 27, 2010 at 05:02:10PM -0400, Chris Metcalf wrote
>> I set aside this thread to look at when I had a minute, and I believe there
>> is just one of the signal issues present in the tile code.  The fix is to
>> reset regs->fault to something other than the "syscall" fault type when
>> exiting from do_signal(), so I'll submit that up for 2.6.37 shortly.
> FWIW, I'd do that in handle_signal() when hitting a syscall restart.

Right now I'm just doing it unconditionally in handle_signal()'s caller
whether or not I actually call handle_signal, to be paranoid:

@@ -353,11 +353,11 @@
                         * clear the TS_RESTORE_SIGMASK flag.
                         */
                        current_thread_info()->status &= ~TS_RESTORE_SIGMASK;
                }

-               return;
+               goto done;
        }

        /* Did we come from a system call? */
        if (regs->faultnum == INT_SWINT_1) {
                /* Restart the system call - no handlers present */
@@ -381,10 +381,14 @@
        /* If there's no signal to deliver, just put the saved sigmask back. */
        if (current_thread_info()->status & TS_RESTORE_SIGMASK) {
                current_thread_info()->status &= ~TS_RESTORE_SIGMASK;
                sigprocmask(SIG_SETMASK, &current->saved_sigmask, NULL);
        }
+
+done:
+       /* Avoid double syscall restart if there are nested signals. */
+       regs->faultnum = INT_SWINT_1_SIGRETURN;
 }
 

> BTW, is everything in your pt_regs safe to modify?

What an interesting observation. :-)  In fact, it would be possible to
overwrite the privilege level (the ex1 register) from within the signal
handler and then return to run arbitrary code at kernel PL.  I'll fix it.

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

WARNING: multiple messages have this Message-ID (diff)
From: Chris Metcalf <cmetcalf@tilera.com>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>, <linux-arch@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: situation with signals
Date: Wed, 27 Oct 2010 18:51:12 -0400	[thread overview]
Message-ID: <4CC8ACE0.1040407@tilera.com> (raw)
In-Reply-To: <20101027213745.GI19804@ZenIV.linux.org.uk>

On 10/27/2010 5:37 PM, Al Viro wrote:
> On Wed, Oct 27, 2010 at 05:02:10PM -0400, Chris Metcalf wrote
>> I set aside this thread to look at when I had a minute, and I believe there
>> is just one of the signal issues present in the tile code.  The fix is to
>> reset regs->fault to something other than the "syscall" fault type when
>> exiting from do_signal(), so I'll submit that up for 2.6.37 shortly.
> FWIW, I'd do that in handle_signal() when hitting a syscall restart.

Right now I'm just doing it unconditionally in handle_signal()'s caller
whether or not I actually call handle_signal, to be paranoid:

@@ -353,11 +353,11 @@
                         * clear the TS_RESTORE_SIGMASK flag.
                         */
                        current_thread_info()->status &= ~TS_RESTORE_SIGMASK;
                }

-               return;
+               goto done;
        }

        /* Did we come from a system call? */
        if (regs->faultnum == INT_SWINT_1) {
                /* Restart the system call - no handlers present */
@@ -381,10 +381,14 @@
        /* If there's no signal to deliver, just put the saved sigmask back. */
        if (current_thread_info()->status & TS_RESTORE_SIGMASK) {
                current_thread_info()->status &= ~TS_RESTORE_SIGMASK;
                sigprocmask(SIG_SETMASK, &current->saved_sigmask, NULL);
        }
+
+done:
+       /* Avoid double syscall restart if there are nested signals. */
+       regs->faultnum = INT_SWINT_1_SIGRETURN;
 }
 

> BTW, is everything in your pt_regs safe to modify?

What an interesting observation. :-)  In fact, it would be possible to
overwrite the privilege level (the ex1 register) from within the signal
handler and then return to run arbitrary code at kernel PL.  I'll fix it.

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com


  reply	other threads:[~2010-10-27 22:51 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-23 22:11 situation with signals Al Viro
2010-09-24 13:33 ` Arnd Bergmann
     [not found] ` <9lYpY1#WLHA.1516@exchange1.tad.internal.tilera.com>
2010-10-27 21:02   ` Chris Metcalf
2010-10-27 21:02     ` Chris Metcalf
2010-10-27 21:37     ` Al Viro
2010-10-27 22:51       ` Chris Metcalf [this message]
2010-10-27 22:51         ` Chris Metcalf
2010-10-27 23:30         ` Al Viro
2010-10-28 19:47           ` [PATCH] arch/tile: don't allow user code to set the PL via ptrace or signal return Chris Metcalf
2010-10-28 19:47             ` Chris Metcalf
2010-10-28 19:03       ` [PATCH] arch/tile: correct double syscall restart for nested signals Chris Metcalf
2010-10-28 19:03         ` Chris Metcalf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4CC8ACE0.1040407@tilera.com \
    --to=cmetcalf@tilera.com \
    --cc=arnd@arndb.de \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.