public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Willy Tarreau <w@1wt.eu>
To: marcelo@kvack.org, Andi Kleen <ak@suse.de>
Cc: linux-kernel@vger.kernel.org, pageexec@freemail.hu
Subject: [PATCH] x86_64: another fix for canonical RIPs during signal handling
Date: Thu, 22 Jun 2006 23:06:57 +0200	[thread overview]
Message-ID: <20060622210657.GA1221@1wt.eu> (raw)


I've been reported by the PaX Team that the following fix left a
small hole :

[PATCH] Always check that RIPs are canonical during signal handling

+	if (regs->rip >= TASK_SIZE && regs->rip < VSYSCALL_START) {
+		regs->rip = 0;
+		return -EFAULT;
+	}

...

+	if (regs->rip >= TASK_SIZE) {
+		if (sig == SIGSEGV)
+			ka->sa.sa_handler = SIG_DFL;
+		regs->rip = 0;
+	}

"the wrong part is regs->rip=0, i guess the intention was to cause a
 SIGSEGV upon returning to userland, but 0 is a valid userland address,
 an application may very well have something mapped there. the correct
 value would be ~0UL as it's guaranteed to fault on linux."

This explanation makes sense, so here's the patch. Andi, would you please
review and confirm ? Thanks in advance.

      Acked-By: Pax Team
      Signed-off-by: Willy Tarreau <w@1wt.eu>

---

 arch/x86_64/kernel/signal.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

a408e3e80854d04915969fe3ae05152bedf99c79
diff --git a/arch/x86_64/kernel/signal.c b/arch/x86_64/kernel/signal.c
index 8bc844f..f3eff11 100644
--- a/arch/x86_64/kernel/signal.c
+++ b/arch/x86_64/kernel/signal.c
@@ -144,7 +144,7 @@ #define COPY(x)		err |= __get_user(regs-
 	COPY(rdx); COPY(rcx); 
 	COPY(rip);
 	if (regs->rip >= TASK_SIZE && regs->rip < VSYSCALL_START) { 
-		regs->rip = 0;
+		regs->rip = ~0UL; /* force the application to fault */
 		return -EFAULT;
 	}
 	COPY(r8);
@@ -361,7 +361,7 @@ #endif
 	if (regs->rip >= TASK_SIZE) { 
 		if (sig == SIGSEGV)
 			ka->sa.sa_handler = SIG_DFL;
-		regs->rip = 0;
+		regs->rip = ~0UL; /* force the application to fault */
 	}
 	regs->cs = __USER_CS;
 	regs->ss = __USER_DS; 
-- 
1.3.3


             reply	other threads:[~2006-06-22 21:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-06-22 21:06 Willy Tarreau [this message]
2006-06-22 21:26 ` [PATCH] x86_64: another fix for canonical RIPs during signal handling Andi Kleen
2006-06-22 21:33   ` Willy Tarreau
2006-06-22 22:17     ` Andi Kleen
     [not found]     ` <449BC808.4174.277D15CF@pageexec.freemail.hu>
2006-06-23 12:29       ` Andi Kleen
     [not found]       ` <449C0616.4382.286F7C8C@pageexec.freemail.hu>
2006-06-23 13:32         ` Willy Tarreau
2006-06-23 13:46           ` Andi Kleen
2006-06-23 13:52             ` Willy Tarreau
2006-06-23 14:51           ` Alan Cox

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=20060622210657.GA1221@1wt.eu \
    --to=w@1wt.eu \
    --cc=ak@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcelo@kvack.org \
    --cc=pageexec@freemail.hu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox