>> From: Theo de Raadt <[email protected]>
>> 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
>
