On 11.10.2018 23:20, Brad Smith wrote: > On Thu, Oct 11, 2018 at 09:31:23PM +0200, Kamil Rytarowski wrote: >> On 11.10.2018 16:25, Brad Smith wrote: >>> On 10/11/2018 5:41 AM, Kamil Rytarowski wrote: >>> >>>> On 11.10.2018 11:36, Peter Maydell wrote: >>>>> On 11 October 2018 at 00:55, Brad Smith <b...@comstyle.com> wrote: >>>>>> 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. >>>>> Hmm. That "automatically growing" part sounds like >>>>> behaviour we definitely do not want for our use case. >>>>> So we're going to need to make this OS-specific :-( >>>>> >>>> I propose to restrict MAP_STACK it to OpenBSD (with a comment in the >>>> code). Once it will be needed by someone else will be able to enable it >>>> for other OSes. >>> >>> I was going to propose doing something like that but you had replied >>> before I did. >>> What sort of comment did you have in mind? >>> >> >> Why do we want it only on OpenBSD and its either unneeded or meaning >> something else on other OSes. >> >> #ifdef __OpenBSD__ >> flags |= MAP_STACK; >> #endif > > How about the following? >
It looks good to me. Reviewed-by: Kamil Rytarowski <n...@gmx.com> > > diff --git a/util/oslib-posix.c b/util/oslib-posix.c > index fbd0dc8c57..7814e61114 100644 > --- a/util/oslib-posix.c > +++ b/util/oslib-posix.c > @@ -596,6 +596,7 @@ pid_t qemu_fork(Error **errp) > void *qemu_alloc_stack(size_t *sz) > { > void *ptr, *guardpage; > + int flags; > #ifdef CONFIG_DEBUG_STACK_USAGE > void *ptr2; > #endif > @@ -610,8 +611,15 @@ void *qemu_alloc_stack(size_t *sz) > /* allocate one extra page for the guard page */ > *sz += pagesz; > > - ptr = mmap(NULL, *sz, PROT_READ | PROT_WRITE, > - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > + flags = MAP_PRIVATE | MAP_ANONYMOUS; > +#if defined(MAP_STACK) && defined(__OpenBSD__) > + /* Only enable MAP_STACK on OpenBSD. Other OS's such > + as Linux/FreeBSD/NetBSD have a flag with the same > + name but have differing functionality. */ > + flags |= MAP_STACK; > +#endif > + > + ptr = mmap(NULL, *sz, PROT_READ | PROT_WRITE, flags, -1, 0); > if (ptr == MAP_FAILED) { > perror("failed to allocate memory for stack"); > abort(); >
signature.asc
Description: OpenPGP digital signature