Subject: Re: delivering faulted-upon address in trap frame to userland
To: None <port-i386@NetBSD.org>
From: Matthias Drochner <M.Drochner@fz-juelich.de>
List: port-i386
Date: 09/02/2006 00:04:21
This is a multipart MIME message.

--==_Exmh_1896384888000
Content-Type: text/plain; charset=us-ascii


M.Drochner@fz-juelich.de said:
> It should be noted that the code implementing this on i386 (and
> probably amd64) is not quite correct

OK, before I completely forget about this I'll try to
describe the problem and a possible fix.

The problem is that SA_SIGINFO expects the address of the
faulting memory reference to be delivered in the SIGSEGV
and SIGBUS cases. And if we can't provide this, it is better
to generate a SIGILL which just requires the address of the faulting
instruction than to deliver wrong data.
We can't provide the fault address of memory references
unless it is a plain page fault. Blame Intel for this, but
it is just a fact. CR2 is only set on page faults, the other
uses in trap.c are wrong. The address of the faulting instruction
is always gathered easily of course.
(We _could_ find out the address of data references, but that would
require emulation techniques which are clearly not worth the effort
here.)

Here is a patch which cleans this up quite radically. I haven't found
bad side effects of this, just good ones, see
http://lists.freedesktop.org/archives/liboil/2006-August/000106.html

There might be bad effects in case programs catch SIGSEGV but get
a SIGILL now, but I didn't find one so far.

What do you think?

best regards
Matthias



--==_Exmh_1896384888000
Content-Type: text/plain ; name="i386trap.txt"; charset=us-ascii
Content-Description: i386trap.txt
Content-Disposition: attachment; filename="i386trap.txt"

Index: arch/i386/i386/trap.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/i386/trap.c,v
retrieving revision 1.213
diff -u -p -r1.213 trap.c
--- arch/i386/i386/trap.c	23 Jul 2006 22:06:05 -0000	1.213
+++ arch/i386/i386/trap.c	1 Sep 2006 15:40:41 -0000
@@ -449,43 +449,17 @@ copyfault:
 		    &l->l_addr->u_pcb)) {
 			goto out;
 		}
-		KSI_INIT_TRAP(&ksi);
-		ksi.ksi_signo = SIGSEGV;
-		ksi.ksi_addr = (void *)rcr2();
-		ksi.ksi_code = SEGV_ACCERR;
-		goto trapsignal;
-
+		/* FALLTHROUGH */
 	case T_TSSFLT|T_USER:
 	case T_SEGNPFLT|T_USER:
 	case T_STKFLT|T_USER:
 	case T_ALIGNFLT|T_USER:
 	case T_NMI|T_USER:
-		KSI_INIT_TRAP(&ksi);
-		ksi.ksi_signo = SIGBUS;
-		ksi.ksi_addr = (void *)rcr2();
-		switch (type) {
-		case T_SEGNPFLT|T_USER:
-		case T_STKFLT|T_USER:
-			ksi.ksi_code = BUS_ADRERR;
-			break;
-		case T_TSSFLT|T_USER:
-		case T_NMI|T_USER:
-			ksi.ksi_code = BUS_OBJERR;
-			break;
-		case T_ALIGNFLT|T_USER:
-			ksi.ksi_code = BUS_ADRALN;
-			break;
-		default:
-			KASSERT(1);
-			break;
-		}
-		goto trapsignal;
-
 	case T_PRIVINFLT|T_USER:	/* privileged instruction fault */
 	case T_FPOPFLT|T_USER:		/* coprocessor operand fault */
 		KSI_INIT_TRAP(&ksi);
 		ksi.ksi_signo = SIGILL;
-		ksi.ksi_addr = (void *)rcr2();
+		ksi.ksi_addr = (void *)frame->tf_eip;
 		switch (type) {
 		case T_PRIVINFLT|T_USER:
 			ksi.ksi_code = ILL_PRVOPC;
@@ -494,7 +468,7 @@ copyfault:
 			ksi.ksi_code = ILL_COPROC;
 			break;
 		default:
-			ksi.ksi_code = 0;
+			ksi.ksi_code = ILL_ILLOPN; /* ??? */
 			break;
 		}
 		goto trapsignal;

--==_Exmh_1896384888000--