Re: [Qemu-devel] [PATCH] oslib-posix: Use MAP_STACK in qemu_alloc_stack() on OpenBSD

[Brad Smith]

On 10/9/2018 11:04 AM, Kamil Rytarowski wrote:

> On 09.10.2018 16:12, Peter Maydell wrote:
> > On 9 October 2018 at 14:52, Kamil Rytarowski <address@hidden> wrote:
> > > On 07.10.2018 17:37, Brad Smith wrote:
> > > > Use MAP_STACK in qemu_alloc_stack() on OpenBSD.
> > > >
> > > > Added to -current and will be in our soon to be 6.4 release.
> > > >
> > > > MAP_STACK      Indicate that the mapping is used as a stack.  This
> > > >                 flag must be used in combination with MAP_ANON and
> > > >                 MAP_PRIVATE.
> > > >
> > > > Implement MAP_STACK option for mmap().  Synchronous faults (pagefault and
> > > > syscall) confirm the stack register points at MAP_STACK memory, otherwise
> > > > SIGSEGV is delivered. sigaltstack() and pthread_attr_setstack() are modified
> > > > to create a MAP_STACK sub-region which satisfies alignment requirements.
> > > > Observe that MAP_STACK can only be set/cleared by mmap(), which zeroes the
> > > > contents of the region -- there is no mprotect() equivalent operation, so
> > > > there is no MAP_STACK-adding gadget.
> > > >
> > > >
> > > > Signed-off-by: Brad Smith <address@hidden>
> > > >
> > > >
> > > > diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> > > > index fbd0dc8c57..51e9a012c2 100644
> > > > --- a/util/oslib-posix.c
> > > > +++ b/util/oslib-posix.c
> > > > @@ -611,7 +611,11 @@ void *qemu_alloc_stack(size_t *sz)
> > > >       *sz += pagesz;
> > > >
> > > >       ptr = mmap(NULL, *sz, PROT_READ | PROT_WRITE,
> > > > -               MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > > > +               MAP_PRIVATE | MAP_ANONYMOUS
> > > > +#ifdef MAP_STACK
> > > > +               | MAP_STACK
> > > > +#endif
> > > > +               , -1, 0);
> > > >       if (ptr == MAP_FAILED) {
> > > >           perror("failed to allocate memory for stack");
> > > >           abort();
> > > Can we handle it differently, storing MAP_* flags in a variable:
> > >
> > > int flags = MAP_PRIVATE | MAP_ANONYMOUS;
> > > #ifdef MAP_STACK
> > > flags |= MAP_STACK;
> > > #endif
> > >
> > > ptr = mmap(NULL, *sz, PROT_READ | PROT_WRITE, flags, -1, 0);
> > >
> > > This way it will look nicer as we won't ifdef the middle of a function call.
> > The other nice way to handle that is to have osdep.h do
> > #ifndef MAP_STACK
> > #define MAP_STACK 0
> > #endif
> > and then you can just unconditionally use MAP_STACK in your
> > expression for the mmap flags.
> I assume that this is a cleaner solution.
>
> > I note that Linux also defines a MAP_STACK, about which the
> > manpage says:
> >         MAP_STACK (since Linux 2.6.27)
> >                Allocate the mapping at an address suitable for a
> > process or thread  stack.
> >                This  flag  is currently a no-op, but is used in the
> > glibc threading imple‐
> >                mentation so that if some architectures require special
> > treatment for stack
> >                allocations, support can later be transparently
> > implemented for glibc.
> >
> > So this patch would be opting Linux QEMU builds into whatever that
> > potential future behaviour change is. That sounds I guess like
> > it's more likely to be the right thing than the wrong thing,
> > but it would be useful if some Linux expert could confirm...
> >
> > thanks
> > -- PMM
> There is a similar description on NetBSD:
>
> MAP_STACK
>
> Allocate a memory segment that can be used
> either for a process or thread stack.  This
> currently has no effect, but its use is
> reserved for architectures that might require
> special treatment of that address space.
> Unimplemented.
>
> Apparently OpenBSD started to overload it for some hardening.

And from FreeBSD...

     MAP_STACK          MAP_STACK implies MAP_ANON, and /offset/  of 0.  The/fd/
                        argument must be -1 and /prot/  must include at least
                        PROT_READ and PROT_WRITE.

                        This option creates a memory region that grows to at
                        most/len/  bytes in size,       starting from the stack 
top
                        and growing down.  The stack top is the starting
                        address returned by the call, plus/len/  bytes.  The
                        bottom of the stack at maximum growth is the starting
                        address returned by the call.

                        Stacks created with MAP_STACK automatically grow.
                        Guards prevent inadvertent use of the regions into
                        which those stacks can grow without requiring mapping
                        the whole stack in advance.