Re: POWER: Unexpected fault when writing to brk-allocated memory

[Nicholas Piggin <npiggin@gmail.com>]

On Mon, 6 Nov 2017 09:32:25 +0100
Florian Weimer <fweimer@redhat.com> wrote:

> On 11/06/2017 09:30 AM, Aneesh Kumar K.V wrote:
> > On 11/06/2017 01:55 PM, Nicholas Piggin wrote:  
> >> On Mon, 6 Nov 2017 09:11:37 +0100
> >> Florian Weimer <fweimer@redhat.com> wrote:
> >>  
> >>> On 11/06/2017 07:47 AM, Nicholas Piggin wrote:  
> >>>> "You get < 128TB unless explicitly requested."
> >>>>
> >>>> Simple, reasonable, obvious rule. Avoids breaking apps that store
> >>>> some bits in the top of pointers (provided that memory allocator
> >>>> userspace libraries also do the right thing).  
> >>>
> >>> So brk would simplify fail instead of crossing the 128 TiB threshold?  
> >>
> >> Yes, that was the intention and that's what x86 seems to do.
> >>  
> >>>
> >>> glibc malloc should cope with that and switch to malloc, but this code
> >>> path is obviously less well-tested than the regular way.  
> >>
> >> Switch to mmap() I guess you meant?  
> 
> Yes, sorry.
> 
> >> powerpc has a couple of bugs in corner cases, so those should be fixed
> >> according to intended policy for stable kernels I think.
> >>
> >> But I question the policy. Just seems like an ugly and ineffective wart.
> >> Exactly for such cases as this -- behaviour would change from run to run
> >> depending on your address space randomization for example! In case your
> >> brk happens to land nicely on 128TB then the next one would succeed.  
> > 
> > Why ? It should not change between run to run. We limit the free
> > area search range based on hint address. So we should get consistent 
> > results across run. even if we changed the context.addr_limit.  
> 
> The size of the gap to the 128 TiB limit varies between runs because of 
> ASLR.  So some runs would use brk alone, others would use brk + malloc. 
> That's not really desirable IMHO.

Yeah. Actually I looked at the code a bit more, and it seems that the
intention is for MAP_FIXED to do exactly what I wanted. brk() uses
MAP_FIXED under the covers, so this case should be okay I think. I'm
just slightly happier now, but I still think it's not the right thing
to do to fail an explicit request for crossing 128TB with a hint. Same
fundamental criticism still applies -- it does not really solve bugs
and just adds an unintuitive wart to the API, and a random change in
behaviour based on randomization.

Anyway I sent some patches that are split up better and hopefully solve
some bugs for powerpc without changing intended policy. That's left for
another discussion.

Thanks,
Nick