Re: signal info code SEGV_ACCERR

[Mark Kettenis]

> Date: Thu, 13 Jul 2017 14:54:41 +0200
> From: Alexander Bluhm 
> 
> Hi,
> 
> The regress test src/regress/sys/kern/siginfo-fault checks wether
> the si_code is set to SEGV_ACCERR after memory access with wrong
> permissions has triggert a SIGSEGV.
> 
> Relevant commit message of the test is:
> According to POSIX, SIGSEGV should specify SEGV_ACCERR if the memory
> pages are mapped, but the protections don't match the user's access
> attempts, while SEGV_MAPERR should only be specified for pages that
> are unmapped.  Some platforms currently handle this correctly, but not
> all.
> 
> This diff changes the behavior of i366 and amd64.  hppa seems to
> do something similar.  Changing error form EACCES to EFAULT in amd64
> affects only a printf, so I removed the useless code.  Also change
> rv to errno in i386 to make it look like amd64.
> 
> ok?

I like this.  ok kettenis@

> Index: arch/amd64/amd64/trap.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/trap.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 trap.c
> --- arch/amd64/amd64/trap.c   30 Apr 2017 13:04:49 -  1.54
> +++ arch/amd64/amd64/trap.c   13 Jul 2017 12:31:28 -
> @@ -362,9 +362,6 @@ faultcommon:
>   KERNEL_UNLOCK();
>   goto out;
>   }
> - if (error == EACCES) {
> - error = EFAULT;
> - }
>  
>   if (type == T_PAGEFLT) {
>   if (pcb->pcb_onfault != 0) {
> @@ -389,7 +386,8 @@ faultcommon:
>   frame_dump(frame);
>  #endif
>   sv.sival_ptr = (void *)fa;
> - trapsignal(p, SIGSEGV, T_PAGEFLT, SEGV_MAPERR, sv);
> + trapsignal(p, SIGSEGV, T_PAGEFLT,
> + error == EACCES ? SEGV_ACCERR : SEGV_MAPERR, sv);
>   }
>   KERNEL_UNLOCK();
>   break;
> Index: arch/i386/i386/trap.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/i386/i386/trap.c,v
> retrieving revision 1.130
> diff -u -p -r1.130 trap.c
> --- arch/i386/i386/trap.c 30 Apr 2017 13:04:49 -  1.130
> +++ arch/i386/i386/trap.c 13 Jul 2017 12:33:30 -
> @@ -373,7 +373,7 @@ trap(struct trapframe *frame)
>   vaddr_t va, fa;
>   struct vmspace *vm;
>   struct vm_map *map;
> - int rv;
> + int error;
>  
>   cr2 = rcr2();
>   KERNEL_LOCK();
> @@ -406,12 +406,12 @@ trap(struct trapframe *frame)
>   if (curcpu()->ci_inatomic == 0 || map == kernel_map) {
>   onfault = p->p_addr->u_pcb.pcb_onfault;
>   p->p_addr->u_pcb.pcb_onfault = NULL;
> - rv = uvm_fault(map, va, 0, ftype);
> + error = uvm_fault(map, va, 0, ftype);
>   p->p_addr->u_pcb.pcb_onfault = onfault;
>   } else
> - rv = EFAULT;
> + error = EFAULT;
>  
> - if (rv == 0) {
> + if (error == 0) {
>   if (map != kernel_map)
>   uvm_grow(p, va);
>   if (type == T_PAGEFLT) {
> @@ -428,11 +428,12 @@ trap(struct trapframe *frame)
>   goto copyfault;
>   }
>   printf("uvm_fault(%p, 0x%lx, 0, %d) -> %x\n",
> - map, va, ftype, rv);
> + map, va, ftype, error);
>   goto we_re_toast;
>   }
>   sv.sival_int = fa;
> - trapsignal(p, SIGSEGV, vftype, SEGV_MAPERR, sv);
> + trapsignal(p, SIGSEGV, vftype,
> + error == EACCES ? SEGV_ACCERR : SEGV_MAPERR, sv);
>   KERNEL_UNLOCK();
>   break;
>   }
> 
> 

signal info code SEGV_ACCERR

[Alexander Bluhm]

Hi,

The regress test src/regress/sys/kern/siginfo-fault checks wether
the si_code is set to SEGV_ACCERR after memory access with wrong
permissions has triggert a SIGSEGV.

Relevant commit message of the test is:
According to POSIX, SIGSEGV should specify SEGV_ACCERR if the memory
pages are mapped, but the protections don't match the user's access
attempts, while SEGV_MAPERR should only be specified for pages that
are unmapped.  Some platforms currently handle this correctly, but not
all.

This diff changes the behavior of i366 and amd64.  hppa seems to
do something similar.  Changing error form EACCES to EFAULT in amd64
affects only a printf, so I removed the useless code.  Also change
rv to errno in i386 to make it look like amd64.

ok?

bluhm

Index: arch/amd64/amd64/trap.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/trap.c,v
retrieving revision 1.54
diff -u -p -r1.54 trap.c
--- arch/amd64/amd64/trap.c 30 Apr 2017 13:04:49 -  1.54
+++ arch/amd64/amd64/trap.c 13 Jul 2017 12:31:28 -
@@ -362,9 +362,6 @@ faultcommon:
KERNEL_UNLOCK();
goto out;
}
-   if (error == EACCES) {
-   error = EFAULT;
-   }
 
if (type == T_PAGEFLT) {
if (pcb->pcb_onfault != 0) {
@@ -389,7 +386,8 @@ faultcommon:
frame_dump(frame);
 #endif
sv.sival_ptr = (void *)fa;
-   trapsignal(p, SIGSEGV, T_PAGEFLT, SEGV_MAPERR, sv);
+   trapsignal(p, SIGSEGV, T_PAGEFLT,
+   error == EACCES ? SEGV_ACCERR : SEGV_MAPERR, sv);
}
KERNEL_UNLOCK();
break;
Index: arch/i386/i386/trap.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/i386/i386/trap.c,v
retrieving revision 1.130
diff -u -p -r1.130 trap.c
--- arch/i386/i386/trap.c   30 Apr 2017 13:04:49 -  1.130
+++ arch/i386/i386/trap.c   13 Jul 2017 12:33:30 -
@@ -373,7 +373,7 @@ trap(struct trapframe *frame)
vaddr_t va, fa;
struct vmspace *vm;
struct vm_map *map;
-   int rv;
+   int error;
 
cr2 = rcr2();
KERNEL_LOCK();
@@ -406,12 +406,12 @@ trap(struct trapframe *frame)
if (curcpu()->ci_inatomic == 0 || map == kernel_map) {
onfault = p->p_addr->u_pcb.pcb_onfault;
p->p_addr->u_pcb.pcb_onfault = NULL;
-   rv = uvm_fault(map, va, 0, ftype);
+   error = uvm_fault(map, va, 0, ftype);
p->p_addr->u_pcb.pcb_onfault = onfault;
} else
-   rv = EFAULT;
+   error = EFAULT;
 
-   if (rv == 0) {
+   if (error == 0) {
if (map != kernel_map)
uvm_grow(p, va);
if (type == T_PAGEFLT) {
@@ -428,11 +428,12 @@ trap(struct trapframe *frame)
goto copyfault;
}
printf("uvm_fault(%p, 0x%lx, 0, %d) -> %x\n",
-   map, va, ftype, rv);
+   map, va, ftype, error);
goto we_re_toast;
}
sv.sival_int = fa;
-   trapsignal(p, SIGSEGV, vftype, SEGV_MAPERR, sv);
+   trapsignal(p, SIGSEGV, vftype,
+   error == EACCES ? SEGV_ACCERR : SEGV_MAPERR, sv);
KERNEL_UNLOCK();
break;
}