>> From: Theo de Raadt <dera...@openbsd.org> >> Date: Thu, 24 Sep 2020 15:27:06 -0600 (MDT) >> >> >The call is missing from the trap handler, probably because I was >> >looking at arm64 where it is missing as well. The result is that the >> >stack size accounting will be wrong. >> >> Nice find. >> >> >In the diff below I only added the call to the "data" trap. That >> >means that an "instruction" trap will not run the accounting code. Is >> >that correct? The uvm_fault() call should never return success in >> >that case unless the stack has been mapped executable... >> >> I think both should have it. munmap and mprotect exist, and >> people can do strange things. > >Which would be this diff. ok?
Yes. Architectures with a single fault routing aren't seperating out exec-faults (for instance x86 PGEX_I faults) and skipping uvm_grow. uvm_grow is detecting things in the "stack zone", and we don't specifically prevent X mappings there. We've pushed back on some toolchain parts that wanted executable stacks very succesfully. W|X in the stack region won't work because of W^X, but R|X would. I'm not sure we can forcibly exclude it. I see little risk in uvm_grow from calling it, and letting it adjust the numbers. It is just accounting.. >Index: arch/powerpc64/powerpc64/trap.c >=================================================================== >RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/trap.c,v >retrieving revision 1.39 >diff -u -p -r1.39 trap.c >--- arch/powerpc64/powerpc64/trap.c 24 Sep 2020 20:22:15 -0000 1.39 >+++ arch/powerpc64/powerpc64/trap.c 24 Sep 2020 21:36:27 -0000 >@@ -181,6 +181,8 @@ trap(struct trapframe *frame) > ftype = PROT_READ; > KERNEL_LOCK(); > error = uvm_fault(map, trunc_page(va), 0, ftype); >+ if (error == 0) >+ uvm_grow(p, trunc_page(va)); > KERNEL_UNLOCK(); > if (error) { > #ifdef TRAP_DEBUG >@@ -225,6 +227,8 @@ trap(struct trapframe *frame) > ftype = PROT_READ | PROT_EXEC; > KERNEL_LOCK(); > error = uvm_fault(map, trunc_page(va), 0, ftype); >+ if (error == 0) >+ uvm_grow(p, trunc_page(va)); > KERNEL_UNLOCK(); > if (error) { > #ifdef TRAP_DEBUG >