map stack problem with "homemade" stackguard

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

map stack problem with "homemade" stackguard

Sebastien Marie-3
Hi,

I recently tried to run qemu, and it failed with SIGABRT and dmesg
contains:

map stack 0x86277000-0x86378000 of map 0xd5343afc failed: bad protection
map stack for pid 43079 failed

I initially think about missing MAP_STACK. And effectively, the flag is
missing on the port (function qemu_alloc_stack() in util/oslib-posix.c).
But in fact, it seems it isn't underline problem of my failure: even
with MAP_STACK, the program is still aborted at the same place.

The real problem is qemu implements a stackguard protection

from qemu, util/oslib-posix.c (with manually added MAP_STACK):
   540      ptr = mmap(NULL, *sz, PROT_READ | PROT_WRITE,
   541                 MAP_PRIVATE | MAP_ANONYMOUS |┬áMAP_STACK, -1, 0);
   542      if (ptr == MAP_FAILED) {
   543          perror("failed to allocate memory for stack");
   544          abort();
   545      }
   546
   547  #if defined(HOST_IA64)
   548      /* separate register stack */
   549      guardpage = ptr + (((*sz - pagesz) / 2) & ~pagesz);
   550  #elif defined(HOST_HPPA)
   551      /* stack grows up */
   552      guardpage = ptr + *sz - pagesz;
   553  #else
   554      /* stack grows down */
   555      guardpage = ptr;
   556  #endif
   557
   558      if (mprotect(guardpage, pagesz, PROT_NONE) != 0) {
   559          perror("failed to set up stack guard page");
   560          abort();
   561      }
   562

and the kernel doesn't like it: in uvm_map_is_stack_remappable(), the
protection check is done by only allowing PROT_READ | PROT_WRITE:

sys/uvm/uvm_map.c
  1852                  if (iter->protection != (PROT_READ | PROT_WRITE)) {
  1853                          printf("map stack 0x%lx-0x%lx of map %p failed: "
  1854                              "bad protection\n", addr, end, map);
  1855                          return FALSE;
  1856                  }

and the guardpage segment is PROT_NONE, resulting an error, and qemu
abort.

If I remove the mprotect(2) call from qemu, the program starts fine (I
didn't test it deeply).

Assuming the check on protection is a bit too restrictive, and it is
fine to have some chunks with PROT_NONE, I think we want something like:

Index: uvm/uvm_map.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_map.c,v
retrieving revision 1.234
diff -u -p -r1.234 uvm_map.c
--- uvm/uvm_map.c 12 Apr 2018 17:13:44 -0000 1.234
+++ uvm/uvm_map.c 16 Apr 2018 15:20:30 -0000
@@ -1849,7 +1849,11 @@ uvm_map_is_stack_remappable(struct vm_ma
 #if 0
  printf("iter prot: 0x%x\n", iter->protection);
 #endif
- if (iter->protection != (PROT_READ | PROT_WRITE)) {
+ switch (iter->protection) {
+ case PROT_READ|PROT_WRITE:
+ case PROT_NONE:
+ break;
+ default:
  printf("map stack 0x%lx-0x%lx of map %p failed: "
     "bad protection\n", addr, end, map);
  return FALSE;


I will send a diff for emulators/qemu to add MAP_STACK after this part
is resolved.

Thanks.
--
Sebastien Marie

Reply | Threaded
Open this post in threaded view
|

Re: map stack problem with "homemade" stackguard

Ted Unangst-6
Sebastien Marie wrote:
> Assuming the check on protection is a bit too restrictive, and it is
> fine to have some chunks with PROT_NONE, I think we want something like:

are readonly stacks meaningful? if so, we should fix that too now instead of
waiting.

>
> Index: uvm/uvm_map.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_map.c,v
> retrieving revision 1.234
> diff -u -p -r1.234 uvm_map.c
> --- uvm/uvm_map.c 12 Apr 2018 17:13:44 -0000 1.234
> +++ uvm/uvm_map.c 16 Apr 2018 15:20:30 -0000
> @@ -1849,7 +1849,11 @@ uvm_map_is_stack_remappable(struct vm_ma
>  #if 0
>   printf("iter prot: 0x%x\n", iter->protection);
>  #endif
> - if (iter->protection != (PROT_READ | PROT_WRITE)) {
> + switch (iter->protection) {
> + case PROT_READ|PROT_WRITE:
> + case PROT_NONE:
> + break;
> + default:
>   printf("map stack 0x%lx-0x%lx of map %p failed: "
>      "bad protection\n", addr, end, map);
>   return FALSE;
>
>
> I will send a diff for emulators/qemu to add MAP_STACK after this part
> is resolved.
>
> Thanks.
> --
> Sebastien Marie
>

Reply | Threaded
Open this post in threaded view
|

Re: map stack problem with "homemade" stackguard

Theo de Raadt-2
Ted Unangst <[hidden email]> wrote:

> Sebastien Marie wrote:
> > Assuming the check on protection is a bit too restrictive, and it is
> > fine to have some chunks with PROT_NONE, I think we want something like:
>
> are readonly stacks meaningful? if so, we should fix that too now instead of
> waiting.

Other systems lack allocators that opportunistically place guard pages
off the edge of mmap/malloc allocations.

That as led some programs to install their own guard-page to catch
small-range stack overflows (useless for large-range reach, as the
public learned a couple months ago).

The full range gets passed to sigaltstack, which freaks out due to a
recently added check included in the MAP_STACK work.  That check isn't
needed since prot checks still get evaluated seperately by uvm_fault

and sigaltstack should always succeed.

So stefan is deleting the check.

What occurs without this check?  The sigaltstack MAP_STACK-overtake will
override the in-region guard-page, turning it into PROT_READ|PROT_WRITE
w/ MAP_STACK.  I have a seperate discussion with stefan about how to
avoid PROT_* mangling that one page inside uvm_map_remap_as_stack(),
basically if the overflow-end of stack is properly aligned, ignore 1
full page in case it is a guard.  Don't make it MAP_STACK, and don't
change it's permissions.

MAP_STACK can therefore act as an opportunistic overflow detector, and
an applications own PROT_NONE guard can act so otherwise.