[PATCH] x86_64: another fix for canonical RIPs during signal handling

[Willy Tarreau]

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@xxxxxx>

---

 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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/