Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Matthew Wilcox]

On Fri, Dec 08, 2017 at 12:13:31PM -0800, Kees Cook wrote:
> On Fri, Dec 8, 2017 at 12:33 AM, Michal Hocko  wrote:
> > OK, this doesn't seem to lead to anywhere. The more this is discussed
> > the more names we are getting. So you know what? I will resubmit and
> > keep my original name. If somebody really hates it then feel free to
> > nack the patch and push alternative and gain concensus on it.
> >
> > I will keep MAP_FIXED_SAFE because it is an alternative to MAP_FIXED so
> > having that in the name is _useful_ for everybody familiar with
> > MAP_FIXED already. And _SAFE suffix tells that the operation doesn't
> > cause any silent memory corruptions or other unexpected side effects.
> 
> Looks like consensus is MAP_FIXED_NOREPLACE.

I'd rather MAP_AT_ADDR or MAP_REQUIRED, but I prefer FIXED_NOREPLACE to
FIXED_SAFE.

I just had a thought though -- MAP_STATIC?  ie don't move it.

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Matthew Wilcox]

On Fri, Dec 08, 2017 at 12:13:31PM -0800, Kees Cook wrote:
> On Fri, Dec 8, 2017 at 12:33 AM, Michal Hocko  wrote:
> > OK, this doesn't seem to lead to anywhere. The more this is discussed
> > the more names we are getting. So you know what? I will resubmit and
> > keep my original name. If somebody really hates it then feel free to
> > nack the patch and push alternative and gain concensus on it.
> >
> > I will keep MAP_FIXED_SAFE because it is an alternative to MAP_FIXED so
> > having that in the name is _useful_ for everybody familiar with
> > MAP_FIXED already. And _SAFE suffix tells that the operation doesn't
> > cause any silent memory corruptions or other unexpected side effects.
> 
> Looks like consensus is MAP_FIXED_NOREPLACE.

I'd rather MAP_AT_ADDR or MAP_REQUIRED, but I prefer FIXED_NOREPLACE to
FIXED_SAFE.

I just had a thought though -- MAP_STATIC?  ie don't move it.

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Florian Weimer]

On 12/08/2017 03:27 PM, Pavel Machek wrote:

On Fri 2017-12-08 22:08:07, Michael Ellerman wrote:

If we had a time machine, the right set of flags would be:

   - MAP_FIXED:   don't treat addr as a hint, fail if addr is not free
   - MAP_REPLACE: replace an existing mapping (or force or clobber)



Actually, if we had a time machine... would we even provide
MAP_REPLACE functionality?


Probably yes.  ELF loading needs to construct a complex set of mappings 
from a single file.  munmap (to create a hole) followed by mmap would be 
racy because another thread could have reused the gap in the meantime. 
The only alternative to overriding existing mappings would be mremap 
with MREMAP_FIXED, and that doesn't look like an improvement API-wise.


(The glibc dynamic linker uses an mmap call with an increased length to 
reserve address space and then loads additional segments with MAP_FIXED 
at the offsets specified in the program header.)


Thanks,
Florian

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Florian Weimer]

On 12/08/2017 03:27 PM, Pavel Machek wrote:

On Fri 2017-12-08 22:08:07, Michael Ellerman wrote:

If we had a time machine, the right set of flags would be:

   - MAP_FIXED:   don't treat addr as a hint, fail if addr is not free
   - MAP_REPLACE: replace an existing mapping (or force or clobber)



Actually, if we had a time machine... would we even provide
MAP_REPLACE functionality?


Probably yes.  ELF loading needs to construct a complex set of mappings 
from a single file.  munmap (to create a hole) followed by mmap would be 
racy because another thread could have reused the gap in the meantime. 
The only alternative to overriding existing mappings would be mremap 
with MREMAP_FIXED, and that doesn't look like an improvement API-wise.


(The glibc dynamic linker uses an mmap call with an increased length to 
reserve address space and then loads additional segments with MAP_FIXED 
at the offsets specified in the program header.)


Thanks,
Florian

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Cyril Hrubis]

Hi!
> > If we had a time machine, the right set of flags would be:
> > 
> >   - MAP_FIXED:   don't treat addr as a hint, fail if addr is not free
> >   - MAP_REPLACE: replace an existing mapping (or force or clobber)
> 
> Actually, if we had a time machine... would we even provide
> MAP_REPLACE functionality?

I did a bit of archeology just beacause we can and since there is a git
repository of the unix history [1].

The first version of mmap() seems to appear in BSD-4_2-Snapshot there was no
MAP_FIXED flag and the addr is expected to be used for the mapping. At least
that is what manual seems to say, the kernel code is not written at this point.
This seems to correspond to a time when Berkley students were busy rewriting
UNIX kernel to take advantage of the VAX's virtual memory.

The MAP_FIXED arrived to the manual shortly after, probably someone figured out
that passing an address to the call does not make much sense in most of the
cases.

The first actual implementation that supports MAP_FIXED appeared in the
BSD-4_3_Reno-Snapshot and already includes the replace behavior. The original
purpose seems to be replacing mappings in the implementation of the execve()
call.

So the answer would probably be yes but it would probably made sense to keep it
as kernel internal flag.

And BTW it looks like HPUX got it right before it was changed to follow POSIX.
There seems to be HPUX compatibility code in the early BSD codebase that
contains both HPUXMAP_FIXED and HPUXMAP_REPLACE.

[1] https://github.com/dspinellis/unix-history-repo

-- 
Cyril Hrubis
chru...@suse.cz

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Cyril Hrubis]

Hi!
> > If we had a time machine, the right set of flags would be:
> > 
> >   - MAP_FIXED:   don't treat addr as a hint, fail if addr is not free
> >   - MAP_REPLACE: replace an existing mapping (or force or clobber)
> 
> Actually, if we had a time machine... would we even provide
> MAP_REPLACE functionality?

I did a bit of archeology just beacause we can and since there is a git
repository of the unix history [1].

The first version of mmap() seems to appear in BSD-4_2-Snapshot there was no
MAP_FIXED flag and the addr is expected to be used for the mapping. At least
that is what manual seems to say, the kernel code is not written at this point.
This seems to correspond to a time when Berkley students were busy rewriting
UNIX kernel to take advantage of the VAX's virtual memory.

The MAP_FIXED arrived to the manual shortly after, probably someone figured out
that passing an address to the call does not make much sense in most of the
cases.

The first actual implementation that supports MAP_FIXED appeared in the
BSD-4_3_Reno-Snapshot and already includes the replace behavior. The original
purpose seems to be replacing mappings in the implementation of the execve()
call.

So the answer would probably be yes but it would probably made sense to keep it
as kernel internal flag.

And BTW it looks like HPUX got it right before it was changed to follow POSIX.
There seems to be HPUX compatibility code in the early BSD codebase that
contains both HPUXMAP_FIXED and HPUXMAP_REPLACE.

[1] https://github.com/dspinellis/unix-history-repo

-- 
Cyril Hrubis
chru...@suse.cz

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Kees Cook]

On Fri, Dec 8, 2017 at 12:33 AM, Michal Hocko  wrote:
> OK, this doesn't seem to lead to anywhere. The more this is discussed
> the more names we are getting. So you know what? I will resubmit and
> keep my original name. If somebody really hates it then feel free to
> nack the patch and push alternative and gain concensus on it.
>
> I will keep MAP_FIXED_SAFE because it is an alternative to MAP_FIXED so
> having that in the name is _useful_ for everybody familiar with
> MAP_FIXED already. And _SAFE suffix tells that the operation doesn't
> cause any silent memory corruptions or other unexpected side effects.

Looks like consensus is MAP_FIXED_NOREPLACE.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Kees Cook]

On Fri, Dec 8, 2017 at 12:33 AM, Michal Hocko  wrote:
> OK, this doesn't seem to lead to anywhere. The more this is discussed
> the more names we are getting. So you know what? I will resubmit and
> keep my original name. If somebody really hates it then feel free to
> nack the patch and push alternative and gain concensus on it.
>
> I will keep MAP_FIXED_SAFE because it is an alternative to MAP_FIXED so
> having that in the name is _useful_ for everybody familiar with
> MAP_FIXED already. And _SAFE suffix tells that the operation doesn't
> cause any silent memory corruptions or other unexpected side effects.

Looks like consensus is MAP_FIXED_NOREPLACE.

-Kees

-- 
Kees Cook
Pixel Security

RE: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[David Laight]

From: Michael Ellerman
> Sent: 08 December 2017 11:08
...
> If we had a time machine, the right set of flags would be:
> 
>   - MAP_FIXED:   don't treat addr as a hint, fail if addr is not free
>   - MAP_REPLACE: replace an existing mapping (or force or clobber)
> 
> But the two were conflated for some reason in the current MAP_FIXED.

Possibly because the original use was loading overlays?

> Given we can't go back and fix it, the closest we can get is to add a
> variant of MAP_FIXED which subtracts the "REPLACE" semantic.
> 
> ie: MAP_FIXED_NOREPLACE

Much better than _SAFE - which is always bad because it is usually
one 'safe' for one specific use case.

David

RE: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[David Laight]

From: Michael Ellerman
> Sent: 08 December 2017 11:08
...
> If we had a time machine, the right set of flags would be:
> 
>   - MAP_FIXED:   don't treat addr as a hint, fail if addr is not free
>   - MAP_REPLACE: replace an existing mapping (or force or clobber)
> 
> But the two were conflated for some reason in the current MAP_FIXED.

Possibly because the original use was loading overlays?

> Given we can't go back and fix it, the closest we can get is to add a
> variant of MAP_FIXED which subtracts the "REPLACE" semantic.
> 
> ie: MAP_FIXED_NOREPLACE

Much better than _SAFE - which is always bad because it is usually
one 'safe' for one specific use case.

David

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Pavel Machek]

On Fri 2017-12-08 22:08:07, Michael Ellerman wrote:
> Matthew Wilcox  writes:
> 
> > On Thu, Dec 07, 2017 at 11:14:27AM -0800, Kees Cook wrote:
> >> On Wed, Dec 6, 2017 at 9:46 PM, Michael Ellerman  
> >> wrote:
> >> > Matthew Wilcox  writes:
> >> >> So, just like we currently say "exactly one of MAP_SHARED or 
> >> >> MAP_PRIVATE",
> >> >> we could add a new paragraph saying "at most one of MAP_FIXED or
> >> >> MAP_REQUIRED" and "any of the following values".
> >> >
> >> > MAP_REQUIRED doesn't immediately grab me, but I don't actively dislike
> >> > it either :)
> >> >
> >> > What about MAP_AT_ADDR ?
> >> >
> >> > It's short, and says what it does on the tin. The first argument to mmap
> >> > is actually called "addr" too.
> >> 
> >> "FIXED" is supposed to do this too.
> >> 
> >> Pavel suggested:
> >> 
> >> MAP_ADD_FIXED
> >> 
> >> (which is different from "use fixed", and describes why it would fail:
> >> can't add since it already exists.)
> >> 
> >> Perhaps "MAP_FIXED_NEW"?
> >> 
> >> There has been a request to drop "FIXED" from the name, so these:
> >> 
> >> MAP_FIXED_NOCLOBBER
> >> MAP_FIXED_NOREPLACE
> >> MAP_FIXED_ADD
> >> MAP_FIXED_NEW
> >> 
> >> Could be:
> >> 
> >> MAP_NOCLOBBER
> >> MAP_NOREPLACE
> >> MAP_ADD
> >> MAP_NEW
> >> 
> >> and we still have the unloved, but acceptable:
> >> 
> >> MAP_REQUIRED
> >> 
> >> My vote is still for "NOREPLACE" or "NOCLOBBER" since it's very
> >> specific, though "NEW" is pretty clear too.
> >
> > How about MAP_NOFORCE?
> 
> It doesn't tell me that addr is not a hint. That's a crucial detail.
> 
> Without MAP_FIXED mmap never "forces/replaces/clobbers", so why would I
> need MAP_NOFORCE if I don't have MAP_FIXED?
> 
> So it needs something in there to indicate that the addr is not a hint,
> that's the only thing that flag actually *does*.
> 
> 
> If we had a time machine, the right set of flags would be:
> 
>   - MAP_FIXED:   don't treat addr as a hint, fail if addr is not free
>   - MAP_REPLACE: replace an existing mapping (or force or clobber)

Actually, if we had a time machine... would we even provide
MAP_REPLACE functionality?

> But the two were conflated for some reason in the current MAP_FIXED.
> 
> Given we can't go back and fix it, the closest we can get is to add a
> variant of MAP_FIXED which subtracts the "REPLACE" semantic.
> 
> ie: MAP_FIXED_NOREPLACE

I like MAP_FIXED_NOREPLACE.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Pavel Machek]

On Fri 2017-12-08 22:08:07, Michael Ellerman wrote:
> Matthew Wilcox  writes:
> 
> > On Thu, Dec 07, 2017 at 11:14:27AM -0800, Kees Cook wrote:
> >> On Wed, Dec 6, 2017 at 9:46 PM, Michael Ellerman  
> >> wrote:
> >> > Matthew Wilcox  writes:
> >> >> So, just like we currently say "exactly one of MAP_SHARED or 
> >> >> MAP_PRIVATE",
> >> >> we could add a new paragraph saying "at most one of MAP_FIXED or
> >> >> MAP_REQUIRED" and "any of the following values".
> >> >
> >> > MAP_REQUIRED doesn't immediately grab me, but I don't actively dislike
> >> > it either :)
> >> >
> >> > What about MAP_AT_ADDR ?
> >> >
> >> > It's short, and says what it does on the tin. The first argument to mmap
> >> > is actually called "addr" too.
> >> 
> >> "FIXED" is supposed to do this too.
> >> 
> >> Pavel suggested:
> >> 
> >> MAP_ADD_FIXED
> >> 
> >> (which is different from "use fixed", and describes why it would fail:
> >> can't add since it already exists.)
> >> 
> >> Perhaps "MAP_FIXED_NEW"?
> >> 
> >> There has been a request to drop "FIXED" from the name, so these:
> >> 
> >> MAP_FIXED_NOCLOBBER
> >> MAP_FIXED_NOREPLACE
> >> MAP_FIXED_ADD
> >> MAP_FIXED_NEW
> >> 
> >> Could be:
> >> 
> >> MAP_NOCLOBBER
> >> MAP_NOREPLACE
> >> MAP_ADD
> >> MAP_NEW
> >> 
> >> and we still have the unloved, but acceptable:
> >> 
> >> MAP_REQUIRED
> >> 
> >> My vote is still for "NOREPLACE" or "NOCLOBBER" since it's very
> >> specific, though "NEW" is pretty clear too.
> >
> > How about MAP_NOFORCE?
> 
> It doesn't tell me that addr is not a hint. That's a crucial detail.
> 
> Without MAP_FIXED mmap never "forces/replaces/clobbers", so why would I
> need MAP_NOFORCE if I don't have MAP_FIXED?
> 
> So it needs something in there to indicate that the addr is not a hint,
> that's the only thing that flag actually *does*.
> 
> 
> If we had a time machine, the right set of flags would be:
> 
>   - MAP_FIXED:   don't treat addr as a hint, fail if addr is not free
>   - MAP_REPLACE: replace an existing mapping (or force or clobber)

Actually, if we had a time machine... would we even provide
MAP_REPLACE functionality?

> But the two were conflated for some reason in the current MAP_FIXED.
> 
> Given we can't go back and fix it, the closest we can get is to add a
> variant of MAP_FIXED which subtracts the "REPLACE" semantic.
> 
> ie: MAP_FIXED_NOREPLACE

I like MAP_FIXED_NOREPLACE.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michael Ellerman]

Matthew Wilcox  writes:

> On Thu, Dec 07, 2017 at 11:14:27AM -0800, Kees Cook wrote:
>> On Wed, Dec 6, 2017 at 9:46 PM, Michael Ellerman  wrote:
>> > Matthew Wilcox  writes:
>> >> So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
>> >> we could add a new paragraph saying "at most one of MAP_FIXED or
>> >> MAP_REQUIRED" and "any of the following values".
>> >
>> > MAP_REQUIRED doesn't immediately grab me, but I don't actively dislike
>> > it either :)
>> >
>> > What about MAP_AT_ADDR ?
>> >
>> > It's short, and says what it does on the tin. The first argument to mmap
>> > is actually called "addr" too.
>> 
>> "FIXED" is supposed to do this too.
>> 
>> Pavel suggested:
>> 
>> MAP_ADD_FIXED
>> 
>> (which is different from "use fixed", and describes why it would fail:
>> can't add since it already exists.)
>> 
>> Perhaps "MAP_FIXED_NEW"?
>> 
>> There has been a request to drop "FIXED" from the name, so these:
>> 
>> MAP_FIXED_NOCLOBBER
>> MAP_FIXED_NOREPLACE
>> MAP_FIXED_ADD
>> MAP_FIXED_NEW
>> 
>> Could be:
>> 
>> MAP_NOCLOBBER
>> MAP_NOREPLACE
>> MAP_ADD
>> MAP_NEW
>> 
>> and we still have the unloved, but acceptable:
>> 
>> MAP_REQUIRED
>> 
>> My vote is still for "NOREPLACE" or "NOCLOBBER" since it's very
>> specific, though "NEW" is pretty clear too.
>
> How about MAP_NOFORCE?

It doesn't tell me that addr is not a hint. That's a crucial detail.

Without MAP_FIXED mmap never "forces/replaces/clobbers", so why would I
need MAP_NOFORCE if I don't have MAP_FIXED?

So it needs something in there to indicate that the addr is not a hint,
that's the only thing that flag actually *does*.


If we had a time machine, the right set of flags would be:

  - MAP_FIXED:   don't treat addr as a hint, fail if addr is not free
  - MAP_REPLACE: replace an existing mapping (or force or clobber)

But the two were conflated for some reason in the current MAP_FIXED.

Given we can't go back and fix it, the closest we can get is to add a
variant of MAP_FIXED which subtracts the "REPLACE" semantic.

ie: MAP_FIXED_NOREPLACE

cheers

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michael Ellerman]

Matthew Wilcox  writes:

> On Thu, Dec 07, 2017 at 11:14:27AM -0800, Kees Cook wrote:
>> On Wed, Dec 6, 2017 at 9:46 PM, Michael Ellerman  wrote:
>> > Matthew Wilcox  writes:
>> >> So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
>> >> we could add a new paragraph saying "at most one of MAP_FIXED or
>> >> MAP_REQUIRED" and "any of the following values".
>> >
>> > MAP_REQUIRED doesn't immediately grab me, but I don't actively dislike
>> > it either :)
>> >
>> > What about MAP_AT_ADDR ?
>> >
>> > It's short, and says what it does on the tin. The first argument to mmap
>> > is actually called "addr" too.
>> 
>> "FIXED" is supposed to do this too.
>> 
>> Pavel suggested:
>> 
>> MAP_ADD_FIXED
>> 
>> (which is different from "use fixed", and describes why it would fail:
>> can't add since it already exists.)
>> 
>> Perhaps "MAP_FIXED_NEW"?
>> 
>> There has been a request to drop "FIXED" from the name, so these:
>> 
>> MAP_FIXED_NOCLOBBER
>> MAP_FIXED_NOREPLACE
>> MAP_FIXED_ADD
>> MAP_FIXED_NEW
>> 
>> Could be:
>> 
>> MAP_NOCLOBBER
>> MAP_NOREPLACE
>> MAP_ADD
>> MAP_NEW
>> 
>> and we still have the unloved, but acceptable:
>> 
>> MAP_REQUIRED
>> 
>> My vote is still for "NOREPLACE" or "NOCLOBBER" since it's very
>> specific, though "NEW" is pretty clear too.
>
> How about MAP_NOFORCE?

It doesn't tell me that addr is not a hint. That's a crucial detail.

Without MAP_FIXED mmap never "forces/replaces/clobbers", so why would I
need MAP_NOFORCE if I don't have MAP_FIXED?

So it needs something in there to indicate that the addr is not a hint,
that's the only thing that flag actually *does*.


If we had a time machine, the right set of flags would be:

  - MAP_FIXED:   don't treat addr as a hint, fail if addr is not free
  - MAP_REPLACE: replace an existing mapping (or force or clobber)

But the two were conflated for some reason in the current MAP_FIXED.

Given we can't go back and fix it, the closest we can get is to add a
variant of MAP_FIXED which subtracts the "REPLACE" semantic.

ie: MAP_FIXED_NOREPLACE

cheers

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

On Thu 07-12-17 11:57:27, Matthew Wilcox wrote:
> On Thu, Dec 07, 2017 at 11:14:27AM -0800, Kees Cook wrote:
> > On Wed, Dec 6, 2017 at 9:46 PM, Michael Ellerman  
> > wrote:
> > > Matthew Wilcox  writes:
> > >> So, just like we currently say "exactly one of MAP_SHARED or 
> > >> MAP_PRIVATE",
> > >> we could add a new paragraph saying "at most one of MAP_FIXED or
> > >> MAP_REQUIRED" and "any of the following values".
> > >
> > > MAP_REQUIRED doesn't immediately grab me, but I don't actively dislike
> > > it either :)
> > >
> > > What about MAP_AT_ADDR ?
> > >
> > > It's short, and says what it does on the tin. The first argument to mmap
> > > is actually called "addr" too.
> > 
> > "FIXED" is supposed to do this too.
> > 
> > Pavel suggested:
> > 
> > MAP_ADD_FIXED
> > 
> > (which is different from "use fixed", and describes why it would fail:
> > can't add since it already exists.)
> > 
> > Perhaps "MAP_FIXED_NEW"?
> > 
> > There has been a request to drop "FIXED" from the name, so these:
> > 
> > MAP_FIXED_NOCLOBBER
> > MAP_FIXED_NOREPLACE
> > MAP_FIXED_ADD
> > MAP_FIXED_NEW
> > 
> > Could be:
> > 
> > MAP_NOCLOBBER
> > MAP_NOREPLACE
> > MAP_ADD
> > MAP_NEW
> > 
> > and we still have the unloved, but acceptable:
> > 
> > MAP_REQUIRED
> > 
> > My vote is still for "NOREPLACE" or "NOCLOBBER" since it's very
> > specific, though "NEW" is pretty clear too.
> 
> How about MAP_NOFORCE?

OK, this doesn't seem to lead to anywhere. The more this is discussed
the more names we are getting. So you know what? I will resubmit and
keep my original name. If somebody really hates it then feel free to
nack the patch and push alternative and gain concensus on it.

I will keep MAP_FIXED_SAFE because it is an alternative to MAP_FIXED so
having that in the name is _useful_ for everybody familiar with
MAP_FIXED already. And _SAFE suffix tells that the operation doesn't
cause any silent memory corruptions or other unexpected side effects.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

On Thu 07-12-17 11:57:27, Matthew Wilcox wrote:
> On Thu, Dec 07, 2017 at 11:14:27AM -0800, Kees Cook wrote:
> > On Wed, Dec 6, 2017 at 9:46 PM, Michael Ellerman  
> > wrote:
> > > Matthew Wilcox  writes:
> > >> So, just like we currently say "exactly one of MAP_SHARED or 
> > >> MAP_PRIVATE",
> > >> we could add a new paragraph saying "at most one of MAP_FIXED or
> > >> MAP_REQUIRED" and "any of the following values".
> > >
> > > MAP_REQUIRED doesn't immediately grab me, but I don't actively dislike
> > > it either :)
> > >
> > > What about MAP_AT_ADDR ?
> > >
> > > It's short, and says what it does on the tin. The first argument to mmap
> > > is actually called "addr" too.
> > 
> > "FIXED" is supposed to do this too.
> > 
> > Pavel suggested:
> > 
> > MAP_ADD_FIXED
> > 
> > (which is different from "use fixed", and describes why it would fail:
> > can't add since it already exists.)
> > 
> > Perhaps "MAP_FIXED_NEW"?
> > 
> > There has been a request to drop "FIXED" from the name, so these:
> > 
> > MAP_FIXED_NOCLOBBER
> > MAP_FIXED_NOREPLACE
> > MAP_FIXED_ADD
> > MAP_FIXED_NEW
> > 
> > Could be:
> > 
> > MAP_NOCLOBBER
> > MAP_NOREPLACE
> > MAP_ADD
> > MAP_NEW
> > 
> > and we still have the unloved, but acceptable:
> > 
> > MAP_REQUIRED
> > 
> > My vote is still for "NOREPLACE" or "NOCLOBBER" since it's very
> > specific, though "NEW" is pretty clear too.
> 
> How about MAP_NOFORCE?

OK, this doesn't seem to lead to anywhere. The more this is discussed
the more names we are getting. So you know what? I will resubmit and
keep my original name. If somebody really hates it then feel free to
nack the patch and push alternative and gain concensus on it.

I will keep MAP_FIXED_SAFE because it is an alternative to MAP_FIXED so
having that in the name is _useful_ for everybody familiar with
MAP_FIXED already. And _SAFE suffix tells that the operation doesn't
cause any silent memory corruptions or other unexpected side effects.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Matthew Wilcox]

On Thu, Dec 07, 2017 at 11:14:27AM -0800, Kees Cook wrote:
> On Wed, Dec 6, 2017 at 9:46 PM, Michael Ellerman  wrote:
> > Matthew Wilcox  writes:
> >> So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
> >> we could add a new paragraph saying "at most one of MAP_FIXED or
> >> MAP_REQUIRED" and "any of the following values".
> >
> > MAP_REQUIRED doesn't immediately grab me, but I don't actively dislike
> > it either :)
> >
> > What about MAP_AT_ADDR ?
> >
> > It's short, and says what it does on the tin. The first argument to mmap
> > is actually called "addr" too.
> 
> "FIXED" is supposed to do this too.
> 
> Pavel suggested:
> 
> MAP_ADD_FIXED
> 
> (which is different from "use fixed", and describes why it would fail:
> can't add since it already exists.)
> 
> Perhaps "MAP_FIXED_NEW"?
> 
> There has been a request to drop "FIXED" from the name, so these:
> 
> MAP_FIXED_NOCLOBBER
> MAP_FIXED_NOREPLACE
> MAP_FIXED_ADD
> MAP_FIXED_NEW
> 
> Could be:
> 
> MAP_NOCLOBBER
> MAP_NOREPLACE
> MAP_ADD
> MAP_NEW
> 
> and we still have the unloved, but acceptable:
> 
> MAP_REQUIRED
> 
> My vote is still for "NOREPLACE" or "NOCLOBBER" since it's very
> specific, though "NEW" is pretty clear too.

How about MAP_NOFORCE?

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Matthew Wilcox]

On Thu, Dec 07, 2017 at 11:14:27AM -0800, Kees Cook wrote:
> On Wed, Dec 6, 2017 at 9:46 PM, Michael Ellerman  wrote:
> > Matthew Wilcox  writes:
> >> So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
> >> we could add a new paragraph saying "at most one of MAP_FIXED or
> >> MAP_REQUIRED" and "any of the following values".
> >
> > MAP_REQUIRED doesn't immediately grab me, but I don't actively dislike
> > it either :)
> >
> > What about MAP_AT_ADDR ?
> >
> > It's short, and says what it does on the tin. The first argument to mmap
> > is actually called "addr" too.
> 
> "FIXED" is supposed to do this too.
> 
> Pavel suggested:
> 
> MAP_ADD_FIXED
> 
> (which is different from "use fixed", and describes why it would fail:
> can't add since it already exists.)
> 
> Perhaps "MAP_FIXED_NEW"?
> 
> There has been a request to drop "FIXED" from the name, so these:
> 
> MAP_FIXED_NOCLOBBER
> MAP_FIXED_NOREPLACE
> MAP_FIXED_ADD
> MAP_FIXED_NEW
> 
> Could be:
> 
> MAP_NOCLOBBER
> MAP_NOREPLACE
> MAP_ADD
> MAP_NEW
> 
> and we still have the unloved, but acceptable:
> 
> MAP_REQUIRED
> 
> My vote is still for "NOREPLACE" or "NOCLOBBER" since it's very
> specific, though "NEW" is pretty clear too.

How about MAP_NOFORCE?

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Kees Cook]

On Wed, Dec 6, 2017 at 9:46 PM, Michael Ellerman  wrote:
> Matthew Wilcox  writes:
>
>> On Tue, Dec 05, 2017 at 08:54:35PM -0800, Matthew Wilcox wrote:
>>> On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
>>> > Cyril Hrubis  writes:
>>> >
>>> > > Hi!
>>> > >> > MAP_FIXED_UNIQUE
>>> > >> > MAP_FIXED_ONCE
>>> > >> > MAP_FIXED_FRESH
>>> > >>
>>> > >> Well, I can open a poll for the best name, but none of those you are
>>> > >> proposing sound much better to me. Yeah, naming sucks...
>>> > >
>>> > > Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
>>> > > would probably be a best fit.
>>> >
>>> > Yeah that could work.
>>> >
>>> > I prefer "no clobber" as I just suggested, because the existing
>>> > MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
>>> > one - which you or another thread may be using - and clobbers it with
>>> > the new one.
>>>
>>> It's longer than MAP_FIXED_WEAK :-P
>>>
>>> You'd have to be pretty darn strong to clobber an existing mapping.
>>
>> I think we're thinking about this all wrong.  We shouldn't document it as
>> "This is a variant of MAP_FIXED".  We should document it as "Here's an
>> alternative to MAP_FIXED".
>>
>> So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
>> we could add a new paragraph saying "at most one of MAP_FIXED or
>> MAP_REQUIRED" and "any of the following values".
>>
>> Now, we should implement MAP_REQUIRED as having each architecture
>> define _MAP_NOT_A_HINT, and then #define MAP_REQUIRED (MAP_FIXED |
>> _MAP_NOT_A_HINT), but that's not information to confuse users with.
>>
>> Also, that lets us add a third option at some point that is Yet Another
>> Way to interpret the 'addr' argument, by having MAP_FIXED clear and
>> _MAP_NOT_A_HINT set.
>>
>> I'm not set on MAP_REQUIRED.  I came up with some awful names
>> (MAP_TODDLER, MAP_TANTRUM, MAP_ULTIMATUM, MAP_BOSS, MAP_PROGRAM_MANAGER,
>> etc).  But I think we should drop FIXED from the middle of the name.
>
> MAP_REQUIRED doesn't immediately grab me, but I don't actively dislike
> it either :)
>
> What about MAP_AT_ADDR ?
>
> It's short, and says what it does on the tin. The first argument to mmap
> is actually called "addr" too.

"FIXED" is supposed to do this too.

Pavel suggested:

MAP_ADD_FIXED

(which is different from "use fixed", and describes why it would fail:
can't add since it already exists.)

Perhaps "MAP_FIXED_NEW"?

There has been a request to drop "FIXED" from the name, so these:

MAP_FIXED_NOCLOBBER
MAP_FIXED_NOREPLACE
MAP_FIXED_ADD
MAP_FIXED_NEW

Could be:

MAP_NOCLOBBER
MAP_NOREPLACE
MAP_ADD
MAP_NEW

and we still have the unloved, but acceptable:

MAP_REQUIRED

My vote is still for "NOREPLACE" or "NOCLOBBER" since it's very
specific, though "NEW" is pretty clear too.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Kees Cook]

On Wed, Dec 6, 2017 at 9:46 PM, Michael Ellerman  wrote:
> Matthew Wilcox  writes:
>
>> On Tue, Dec 05, 2017 at 08:54:35PM -0800, Matthew Wilcox wrote:
>>> On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
>>> > Cyril Hrubis  writes:
>>> >
>>> > > Hi!
>>> > >> > MAP_FIXED_UNIQUE
>>> > >> > MAP_FIXED_ONCE
>>> > >> > MAP_FIXED_FRESH
>>> > >>
>>> > >> Well, I can open a poll for the best name, but none of those you are
>>> > >> proposing sound much better to me. Yeah, naming sucks...
>>> > >
>>> > > Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
>>> > > would probably be a best fit.
>>> >
>>> > Yeah that could work.
>>> >
>>> > I prefer "no clobber" as I just suggested, because the existing
>>> > MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
>>> > one - which you or another thread may be using - and clobbers it with
>>> > the new one.
>>>
>>> It's longer than MAP_FIXED_WEAK :-P
>>>
>>> You'd have to be pretty darn strong to clobber an existing mapping.
>>
>> I think we're thinking about this all wrong.  We shouldn't document it as
>> "This is a variant of MAP_FIXED".  We should document it as "Here's an
>> alternative to MAP_FIXED".
>>
>> So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
>> we could add a new paragraph saying "at most one of MAP_FIXED or
>> MAP_REQUIRED" and "any of the following values".
>>
>> Now, we should implement MAP_REQUIRED as having each architecture
>> define _MAP_NOT_A_HINT, and then #define MAP_REQUIRED (MAP_FIXED |
>> _MAP_NOT_A_HINT), but that's not information to confuse users with.
>>
>> Also, that lets us add a third option at some point that is Yet Another
>> Way to interpret the 'addr' argument, by having MAP_FIXED clear and
>> _MAP_NOT_A_HINT set.
>>
>> I'm not set on MAP_REQUIRED.  I came up with some awful names
>> (MAP_TODDLER, MAP_TANTRUM, MAP_ULTIMATUM, MAP_BOSS, MAP_PROGRAM_MANAGER,
>> etc).  But I think we should drop FIXED from the middle of the name.
>
> MAP_REQUIRED doesn't immediately grab me, but I don't actively dislike
> it either :)
>
> What about MAP_AT_ADDR ?
>
> It's short, and says what it does on the tin. The first argument to mmap
> is actually called "addr" too.

"FIXED" is supposed to do this too.

Pavel suggested:

MAP_ADD_FIXED

(which is different from "use fixed", and describes why it would fail:
can't add since it already exists.)

Perhaps "MAP_FIXED_NEW"?

There has been a request to drop "FIXED" from the name, so these:

MAP_FIXED_NOCLOBBER
MAP_FIXED_NOREPLACE
MAP_FIXED_ADD
MAP_FIXED_NEW

Could be:

MAP_NOCLOBBER
MAP_NOREPLACE
MAP_ADD
MAP_NEW

and we still have the unloved, but acceptable:

MAP_REQUIRED

My vote is still for "NOREPLACE" or "NOCLOBBER" since it's very
specific, though "NEW" is pretty clear too.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michael Ellerman]

Matthew Wilcox  writes:

> On Tue, Dec 05, 2017 at 08:54:35PM -0800, Matthew Wilcox wrote:
>> On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
>> > Cyril Hrubis  writes:
>> > 
>> > > Hi!
>> > >> > MAP_FIXED_UNIQUE
>> > >> > MAP_FIXED_ONCE
>> > >> > MAP_FIXED_FRESH
>> > >> 
>> > >> Well, I can open a poll for the best name, but none of those you are
>> > >> proposing sound much better to me. Yeah, naming sucks...
>> > >
>> > > Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
>> > > would probably be a best fit.
>> > 
>> > Yeah that could work.
>> > 
>> > I prefer "no clobber" as I just suggested, because the existing
>> > MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
>> > one - which you or another thread may be using - and clobbers it with
>> > the new one.
>> 
>> It's longer than MAP_FIXED_WEAK :-P
>> 
>> You'd have to be pretty darn strong to clobber an existing mapping.
>
> I think we're thinking about this all wrong.  We shouldn't document it as
> "This is a variant of MAP_FIXED".  We should document it as "Here's an
> alternative to MAP_FIXED".
>
> So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
> we could add a new paragraph saying "at most one of MAP_FIXED or
> MAP_REQUIRED" and "any of the following values".
>
> Now, we should implement MAP_REQUIRED as having each architecture
> define _MAP_NOT_A_HINT, and then #define MAP_REQUIRED (MAP_FIXED |
> _MAP_NOT_A_HINT), but that's not information to confuse users with.
>
> Also, that lets us add a third option at some point that is Yet Another
> Way to interpret the 'addr' argument, by having MAP_FIXED clear and
> _MAP_NOT_A_HINT set.
>
> I'm not set on MAP_REQUIRED.  I came up with some awful names
> (MAP_TODDLER, MAP_TANTRUM, MAP_ULTIMATUM, MAP_BOSS, MAP_PROGRAM_MANAGER,
> etc).  But I think we should drop FIXED from the middle of the name.

MAP_REQUIRED doesn't immediately grab me, but I don't actively dislike
it either :)

What about MAP_AT_ADDR ?

It's short, and says what it does on the tin. The first argument to mmap
is actually called "addr" too.

cheers

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michael Ellerman]

Matthew Wilcox  writes:

> On Tue, Dec 05, 2017 at 08:54:35PM -0800, Matthew Wilcox wrote:
>> On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
>> > Cyril Hrubis  writes:
>> > 
>> > > Hi!
>> > >> > MAP_FIXED_UNIQUE
>> > >> > MAP_FIXED_ONCE
>> > >> > MAP_FIXED_FRESH
>> > >> 
>> > >> Well, I can open a poll for the best name, but none of those you are
>> > >> proposing sound much better to me. Yeah, naming sucks...
>> > >
>> > > Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
>> > > would probably be a best fit.
>> > 
>> > Yeah that could work.
>> > 
>> > I prefer "no clobber" as I just suggested, because the existing
>> > MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
>> > one - which you or another thread may be using - and clobbers it with
>> > the new one.
>> 
>> It's longer than MAP_FIXED_WEAK :-P
>> 
>> You'd have to be pretty darn strong to clobber an existing mapping.
>
> I think we're thinking about this all wrong.  We shouldn't document it as
> "This is a variant of MAP_FIXED".  We should document it as "Here's an
> alternative to MAP_FIXED".
>
> So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
> we could add a new paragraph saying "at most one of MAP_FIXED or
> MAP_REQUIRED" and "any of the following values".
>
> Now, we should implement MAP_REQUIRED as having each architecture
> define _MAP_NOT_A_HINT, and then #define MAP_REQUIRED (MAP_FIXED |
> _MAP_NOT_A_HINT), but that's not information to confuse users with.
>
> Also, that lets us add a third option at some point that is Yet Another
> Way to interpret the 'addr' argument, by having MAP_FIXED clear and
> _MAP_NOT_A_HINT set.
>
> I'm not set on MAP_REQUIRED.  I came up with some awful names
> (MAP_TODDLER, MAP_TANTRUM, MAP_ULTIMATUM, MAP_BOSS, MAP_PROGRAM_MANAGER,
> etc).  But I think we should drop FIXED from the middle of the name.

MAP_REQUIRED doesn't immediately grab me, but I don't actively dislike
it either :)

What about MAP_AT_ADDR ?

It's short, and says what it does on the tin. The first argument to mmap
is actually called "addr" too.

cheers

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[John Hubbard]

On 12/06/2017 04:19 PM, Kees Cook wrote:
> On Wed, Dec 6, 2017 at 1:08 AM, Michal Hocko  wrote:
>> On Wed 06-12-17 08:33:37, Rasmus Villemoes wrote:
>>> On 2017-12-06 05:50, Michael Ellerman wrote:
 Michal Hocko  writes:

> On Wed 29-11-17 14:25:36, Kees Cook wrote:
> It is safe in a sense it doesn't perform any address space dangerous
> operations. mmap is _inherently_ about the address space so the context
> should be kind of clear.

 So now you have to define what "dangerous" means.

>> MAP_FIXED_UNIQUE
>> MAP_FIXED_ONCE
>> MAP_FIXED_FRESH
>
> Well, I can open a poll for the best name, but none of those you are
> proposing sound much better to me. Yeah, naming sucks...
>>>
>>> I also don't like the _SAFE name - MAP_FIXED in itself isn't unsafe [1],
>>> but I do agree that having a way to avoid clobbering (parts of) an
>>> existing mapping is quite useful. Since we're bikeshedding names, how
>>> about MAP_FIXED_EXCL, in analogy with the O_ flag.
>>
>> I really give up on the name discussion. I will take whatever the
>> majority comes up with. I just do not want this (useful) funtionality
>> get bikeched to death.
> 
> Yup, I really want this to land too. What do people think of Matthew
> Wilcox's MAP_REQUIRED ? MAP_EXACT isn't exact, and dropping "FIXED"
> out of the middle seems sensible to me.

+1, MAP_REQUIRED does sound like the best one so far, yes. Sorry if I 
contributed
to any excessive bikeshedding. :)

thanks,
john h

> 
> MIchael, any suggestions with your API hat on?
> 
> -Kees
> 

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[John Hubbard]

On 12/06/2017 04:19 PM, Kees Cook wrote:
> On Wed, Dec 6, 2017 at 1:08 AM, Michal Hocko  wrote:
>> On Wed 06-12-17 08:33:37, Rasmus Villemoes wrote:
>>> On 2017-12-06 05:50, Michael Ellerman wrote:
 Michal Hocko  writes:

> On Wed 29-11-17 14:25:36, Kees Cook wrote:
> It is safe in a sense it doesn't perform any address space dangerous
> operations. mmap is _inherently_ about the address space so the context
> should be kind of clear.

 So now you have to define what "dangerous" means.

>> MAP_FIXED_UNIQUE
>> MAP_FIXED_ONCE
>> MAP_FIXED_FRESH
>
> Well, I can open a poll for the best name, but none of those you are
> proposing sound much better to me. Yeah, naming sucks...
>>>
>>> I also don't like the _SAFE name - MAP_FIXED in itself isn't unsafe [1],
>>> but I do agree that having a way to avoid clobbering (parts of) an
>>> existing mapping is quite useful. Since we're bikeshedding names, how
>>> about MAP_FIXED_EXCL, in analogy with the O_ flag.
>>
>> I really give up on the name discussion. I will take whatever the
>> majority comes up with. I just do not want this (useful) funtionality
>> get bikeched to death.
> 
> Yup, I really want this to land too. What do people think of Matthew
> Wilcox's MAP_REQUIRED ? MAP_EXACT isn't exact, and dropping "FIXED"
> out of the middle seems sensible to me.

+1, MAP_REQUIRED does sound like the best one so far, yes. Sorry if I 
contributed
to any excessive bikeshedding. :)

thanks,
john h

> 
> MIchael, any suggestions with your API hat on?
> 
> -Kees
> 

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Kees Cook]

On Wed, Dec 6, 2017 at 1:08 AM, Michal Hocko  wrote:
> On Wed 06-12-17 08:33:37, Rasmus Villemoes wrote:
>> On 2017-12-06 05:50, Michael Ellerman wrote:
>> > Michal Hocko  writes:
>> >
>> >> On Wed 29-11-17 14:25:36, Kees Cook wrote:
>> >> It is safe in a sense it doesn't perform any address space dangerous
>> >> operations. mmap is _inherently_ about the address space so the context
>> >> should be kind of clear.
>> >
>> > So now you have to define what "dangerous" means.
>> >
>> >>> MAP_FIXED_UNIQUE
>> >>> MAP_FIXED_ONCE
>> >>> MAP_FIXED_FRESH
>> >>
>> >> Well, I can open a poll for the best name, but none of those you are
>> >> proposing sound much better to me. Yeah, naming sucks...
>>
>> I also don't like the _SAFE name - MAP_FIXED in itself isn't unsafe [1],
>> but I do agree that having a way to avoid clobbering (parts of) an
>> existing mapping is quite useful. Since we're bikeshedding names, how
>> about MAP_FIXED_EXCL, in analogy with the O_ flag.
>
> I really give up on the name discussion. I will take whatever the
> majority comes up with. I just do not want this (useful) funtionality
> get bikeched to death.

Yup, I really want this to land too. What do people think of Matthew
Wilcox's MAP_REQUIRED ? MAP_EXACT isn't exact, and dropping "FIXED"
out of the middle seems sensible to me.

MIchael, any suggestions with your API hat on?

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Kees Cook]

On Wed, Dec 6, 2017 at 1:08 AM, Michal Hocko  wrote:
> On Wed 06-12-17 08:33:37, Rasmus Villemoes wrote:
>> On 2017-12-06 05:50, Michael Ellerman wrote:
>> > Michal Hocko  writes:
>> >
>> >> On Wed 29-11-17 14:25:36, Kees Cook wrote:
>> >> It is safe in a sense it doesn't perform any address space dangerous
>> >> operations. mmap is _inherently_ about the address space so the context
>> >> should be kind of clear.
>> >
>> > So now you have to define what "dangerous" means.
>> >
>> >>> MAP_FIXED_UNIQUE
>> >>> MAP_FIXED_ONCE
>> >>> MAP_FIXED_FRESH
>> >>
>> >> Well, I can open a poll for the best name, but none of those you are
>> >> proposing sound much better to me. Yeah, naming sucks...
>>
>> I also don't like the _SAFE name - MAP_FIXED in itself isn't unsafe [1],
>> but I do agree that having a way to avoid clobbering (parts of) an
>> existing mapping is quite useful. Since we're bikeshedding names, how
>> about MAP_FIXED_EXCL, in analogy with the O_ flag.
>
> I really give up on the name discussion. I will take whatever the
> majority comes up with. I just do not want this (useful) funtionality
> get bikeched to death.

Yup, I really want this to land too. What do people think of Matthew
Wilcox's MAP_REQUIRED ? MAP_EXACT isn't exact, and dropping "FIXED"
out of the middle seems sensible to me.

MIchael, any suggestions with your API hat on?

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

On Wed 06-12-17 08:33:37, Rasmus Villemoes wrote:
> On 2017-12-06 05:50, Michael Ellerman wrote:
> > Michal Hocko  writes:
> > 
> >> On Wed 29-11-17 14:25:36, Kees Cook wrote:
> >> It is safe in a sense it doesn't perform any address space dangerous
> >> operations. mmap is _inherently_ about the address space so the context
> >> should be kind of clear.
> > 
> > So now you have to define what "dangerous" means.
> > 
> >>> MAP_FIXED_UNIQUE
> >>> MAP_FIXED_ONCE
> >>> MAP_FIXED_FRESH
> >>
> >> Well, I can open a poll for the best name, but none of those you are
> >> proposing sound much better to me. Yeah, naming sucks...
> 
> I also don't like the _SAFE name - MAP_FIXED in itself isn't unsafe [1],
> but I do agree that having a way to avoid clobbering (parts of) an
> existing mapping is quite useful. Since we're bikeshedding names, how
> about MAP_FIXED_EXCL, in analogy with the O_ flag.

I really give up on the name discussion. I will take whatever the
majority comes up with. I just do not want this (useful) funtionality
get bikeched to death.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

On Wed 06-12-17 08:33:37, Rasmus Villemoes wrote:
> On 2017-12-06 05:50, Michael Ellerman wrote:
> > Michal Hocko  writes:
> > 
> >> On Wed 29-11-17 14:25:36, Kees Cook wrote:
> >> It is safe in a sense it doesn't perform any address space dangerous
> >> operations. mmap is _inherently_ about the address space so the context
> >> should be kind of clear.
> > 
> > So now you have to define what "dangerous" means.
> > 
> >>> MAP_FIXED_UNIQUE
> >>> MAP_FIXED_ONCE
> >>> MAP_FIXED_FRESH
> >>
> >> Well, I can open a poll for the best name, but none of those you are
> >> proposing sound much better to me. Yeah, naming sucks...
> 
> I also don't like the _SAFE name - MAP_FIXED in itself isn't unsafe [1],
> but I do agree that having a way to avoid clobbering (parts of) an
> existing mapping is quite useful. Since we're bikeshedding names, how
> about MAP_FIXED_EXCL, in analogy with the O_ flag.

I really give up on the name discussion. I will take whatever the
majority comes up with. I just do not want this (useful) funtionality
get bikeched to death.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Florian Weimer]

On 12/06/2017 09:06 AM, John Hubbard wrote:

On 12/05/2017 11:35 PM, Florian Weimer wrote:

On 12/06/2017 08:33 AM, John Hubbard wrote:

In that case, maybe:

  MAP_EXACT

? ...because that's the characteristic behavior.


Is that true?  mmap still silently rounding up the length to the page size, I 
assume, so even that name is misleading.


Hi Florian,

Not as far as I can tell, it's not doing that.

For both MAP_FIXED, and this new flag, the documented (and actual)
behavior is *not* to do any such rounding. Instead, the requested
input address is required to be page-aligned itself, and mmap()
should be honoring the exact addr.


I meant the length, not the address.

Florian

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Florian Weimer]

On 12/06/2017 09:06 AM, John Hubbard wrote:

On 12/05/2017 11:35 PM, Florian Weimer wrote:

On 12/06/2017 08:33 AM, John Hubbard wrote:

In that case, maybe:

  MAP_EXACT

? ...because that's the characteristic behavior.


Is that true?  mmap still silently rounding up the length to the page size, I 
assume, so even that name is misleading.


Hi Florian,

Not as far as I can tell, it's not doing that.

For both MAP_FIXED, and this new flag, the documented (and actual)
behavior is *not* to do any such rounding. Instead, the requested
input address is required to be page-aligned itself, and mmap()
should be honoring the exact addr.


I meant the length, not the address.

Florian

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[John Hubbard]

On 12/05/2017 11:35 PM, Florian Weimer wrote:
> On 12/06/2017 08:33 AM, John Hubbard wrote:
>> In that case, maybe:
>>
>>  MAP_EXACT
>>
>> ? ...because that's the characteristic behavior.
> 
> Is that true?  mmap still silently rounding up the length to the page size, I 
> assume, so even that name is misleading.

Hi Florian,

Not as far as I can tell, it's not doing that.

For both MAP_FIXED, and this new flag, the documented (and actual)
behavior is *not* to do any such rounding. Instead, the requested
input address is required to be page-aligned itself, and mmap()
should be honoring the exact addr.

>From the mmap(2) man page:

   MAP_FIXED
  Don't  interpret  addr  as  a  hint: place the mapping at
  exactly that address.  addr must be  a  multiple  of  the
  page  size. 


And from what I can see, the do_mmap() implementation leaves addr
unchanged, in the MAP_FIXED case:

do_mmap(...)
{
/* ... */
if (!(flags & MAP_FIXED))
addr = round_hint_to_min(addr);

...although it does look like device drivers have the opportunity
to break that:

mmap_region(...)
{
/* Can addr have changed??
 *
 * Answer: Yes, several device drivers can do it in their
 * f_op->mmap method. -DaveM
 * Bug: If addr is changed, prev, rb_link, rb_parent should
 *  be updated for vma_link()
 */
WARN_ON_ONCE(addr != vma->vm_start);

addr = vma->vm_start;
   

--
thanks,
John Hubbard
NVIDIA

> 
> Thanks,
> Florian

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[John Hubbard]

On 12/05/2017 11:35 PM, Florian Weimer wrote:
> On 12/06/2017 08:33 AM, John Hubbard wrote:
>> In that case, maybe:
>>
>>  MAP_EXACT
>>
>> ? ...because that's the characteristic behavior.
> 
> Is that true?  mmap still silently rounding up the length to the page size, I 
> assume, so even that name is misleading.

Hi Florian,

Not as far as I can tell, it's not doing that.

For both MAP_FIXED, and this new flag, the documented (and actual)
behavior is *not* to do any such rounding. Instead, the requested
input address is required to be page-aligned itself, and mmap()
should be honoring the exact addr.

>From the mmap(2) man page:

   MAP_FIXED
  Don't  interpret  addr  as  a  hint: place the mapping at
  exactly that address.  addr must be  a  multiple  of  the
  page  size. 


And from what I can see, the do_mmap() implementation leaves addr
unchanged, in the MAP_FIXED case:

do_mmap(...)
{
/* ... */
if (!(flags & MAP_FIXED))
addr = round_hint_to_min(addr);

...although it does look like device drivers have the opportunity
to break that:

mmap_region(...)
{
/* Can addr have changed??
 *
 * Answer: Yes, several device drivers can do it in their
 * f_op->mmap method. -DaveM
 * Bug: If addr is changed, prev, rb_link, rb_parent should
 *  be updated for vma_link()
 */
WARN_ON_ONCE(addr != vma->vm_start);

addr = vma->vm_start;
   

--
thanks,
John Hubbard
NVIDIA

> 
> Thanks,
> Florian

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Florian Weimer]

On 12/06/2017 08:33 AM, John Hubbard wrote:

In that case, maybe:

 MAP_EXACT

? ...because that's the characteristic behavior.


Is that true?  mmap still silently rounding up the length to the page 
size, I assume, so even that name is misleading.


Thanks,
Florian

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Florian Weimer]

On 12/06/2017 08:33 AM, John Hubbard wrote:

In that case, maybe:

 MAP_EXACT

? ...because that's the characteristic behavior.


Is that true?  mmap still silently rounding up the length to the page 
size, I assume, so even that name is misleading.


Thanks,
Florian

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[John Hubbard]

On 12/05/2017 11:03 PM, Matthew Wilcox wrote:
> On Tue, Dec 05, 2017 at 08:54:35PM -0800, Matthew Wilcox wrote:
>> On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
>>> Cyril Hrubis  writes:
>>>
 Hi!
>> MAP_FIXED_UNIQUE
>> MAP_FIXED_ONCE
>> MAP_FIXED_FRESH
>
> Well, I can open a poll for the best name, but none of those you are
> proposing sound much better to me. Yeah, naming sucks...

 Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
 would probably be a best fit.
>>>
>>> Yeah that could work.
>>>
>>> I prefer "no clobber" as I just suggested, because the existing
>>> MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
>>> one - which you or another thread may be using - and clobbers it with
>>> the new one.
>>
>> It's longer than MAP_FIXED_WEAK :-P
>>
>> You'd have to be pretty darn strong to clobber an existing mapping.
> 
> I think we're thinking about this all wrong.  We shouldn't document it as
> "This is a variant of MAP_FIXED".  We should document it as "Here's an
> alternative to MAP_FIXED".
> 
> So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
> we could add a new paragraph saying "at most one of MAP_FIXED or
> MAP_REQUIRED" and "any of the following values".
> 
> Now, we should implement MAP_REQUIRED as having each architecture
> define _MAP_NOT_A_HINT, and then #define MAP_REQUIRED (MAP_FIXED |
> _MAP_NOT_A_HINT), but that's not information to confuse users with.
> 
> Also, that lets us add a third option at some point that is Yet Another
> Way to interpret the 'addr' argument, by having MAP_FIXED clear and
> _MAP_NOT_A_HINT set.
> 
> I'm not set on MAP_REQUIRED.  I came up with some awful names
> (MAP_TODDLER, MAP_TANTRUM, MAP_ULTIMATUM, MAP_BOSS, MAP_PROGRAM_MANAGER,
> etc).  But I think we should drop FIXED from the middle of the name.
> 

In that case, maybe:

MAP_EXACT

? ...because that's the characteristic behavior. It doesn't clobber, but
you don't need to say that in the name, now that we're not including
_FIXED_ in the middle.

thanks,
John Hubbard
NVIDIA

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[John Hubbard]

On 12/05/2017 11:03 PM, Matthew Wilcox wrote:
> On Tue, Dec 05, 2017 at 08:54:35PM -0800, Matthew Wilcox wrote:
>> On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
>>> Cyril Hrubis  writes:
>>>
 Hi!
>> MAP_FIXED_UNIQUE
>> MAP_FIXED_ONCE
>> MAP_FIXED_FRESH
>
> Well, I can open a poll for the best name, but none of those you are
> proposing sound much better to me. Yeah, naming sucks...

 Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
 would probably be a best fit.
>>>
>>> Yeah that could work.
>>>
>>> I prefer "no clobber" as I just suggested, because the existing
>>> MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
>>> one - which you or another thread may be using - and clobbers it with
>>> the new one.
>>
>> It's longer than MAP_FIXED_WEAK :-P
>>
>> You'd have to be pretty darn strong to clobber an existing mapping.
> 
> I think we're thinking about this all wrong.  We shouldn't document it as
> "This is a variant of MAP_FIXED".  We should document it as "Here's an
> alternative to MAP_FIXED".
> 
> So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
> we could add a new paragraph saying "at most one of MAP_FIXED or
> MAP_REQUIRED" and "any of the following values".
> 
> Now, we should implement MAP_REQUIRED as having each architecture
> define _MAP_NOT_A_HINT, and then #define MAP_REQUIRED (MAP_FIXED |
> _MAP_NOT_A_HINT), but that's not information to confuse users with.
> 
> Also, that lets us add a third option at some point that is Yet Another
> Way to interpret the 'addr' argument, by having MAP_FIXED clear and
> _MAP_NOT_A_HINT set.
> 
> I'm not set on MAP_REQUIRED.  I came up with some awful names
> (MAP_TODDLER, MAP_TANTRUM, MAP_ULTIMATUM, MAP_BOSS, MAP_PROGRAM_MANAGER,
> etc).  But I think we should drop FIXED from the middle of the name.
> 

In that case, maybe:

MAP_EXACT

? ...because that's the characteristic behavior. It doesn't clobber, but
you don't need to say that in the name, now that we're not including
_FIXED_ in the middle.

thanks,
John Hubbard
NVIDIA

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Rasmus Villemoes]

On 2017-12-06 05:50, Michael Ellerman wrote:
> Michal Hocko  writes:
> 
>> On Wed 29-11-17 14:25:36, Kees Cook wrote:
>> It is safe in a sense it doesn't perform any address space dangerous
>> operations. mmap is _inherently_ about the address space so the context
>> should be kind of clear.
> 
> So now you have to define what "dangerous" means.
> 
>>> MAP_FIXED_UNIQUE
>>> MAP_FIXED_ONCE
>>> MAP_FIXED_FRESH
>>
>> Well, I can open a poll for the best name, but none of those you are
>> proposing sound much better to me. Yeah, naming sucks...

I also don't like the _SAFE name - MAP_FIXED in itself isn't unsafe [1],
but I do agree that having a way to avoid clobbering (parts of) an
existing mapping is quite useful. Since we're bikeshedding names, how
about MAP_FIXED_EXCL, in analogy with the O_ flag.

[1] I like the analogy between MAP_FIXED and dup2 made in
.

Rasmus

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Rasmus Villemoes]

On 2017-12-06 05:50, Michael Ellerman wrote:
> Michal Hocko  writes:
> 
>> On Wed 29-11-17 14:25:36, Kees Cook wrote:
>> It is safe in a sense it doesn't perform any address space dangerous
>> operations. mmap is _inherently_ about the address space so the context
>> should be kind of clear.
> 
> So now you have to define what "dangerous" means.
> 
>>> MAP_FIXED_UNIQUE
>>> MAP_FIXED_ONCE
>>> MAP_FIXED_FRESH
>>
>> Well, I can open a poll for the best name, but none of those you are
>> proposing sound much better to me. Yeah, naming sucks...

I also don't like the _SAFE name - MAP_FIXED in itself isn't unsafe [1],
but I do agree that having a way to avoid clobbering (parts of) an
existing mapping is quite useful. Since we're bikeshedding names, how
about MAP_FIXED_EXCL, in analogy with the O_ flag.

[1] I like the analogy between MAP_FIXED and dup2 made in
.

Rasmus

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Matthew Wilcox]

On Tue, Dec 05, 2017 at 08:54:35PM -0800, Matthew Wilcox wrote:
> On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
> > Cyril Hrubis  writes:
> > 
> > > Hi!
> > >> > MAP_FIXED_UNIQUE
> > >> > MAP_FIXED_ONCE
> > >> > MAP_FIXED_FRESH
> > >> 
> > >> Well, I can open a poll for the best name, but none of those you are
> > >> proposing sound much better to me. Yeah, naming sucks...
> > >
> > > Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
> > > would probably be a best fit.
> > 
> > Yeah that could work.
> > 
> > I prefer "no clobber" as I just suggested, because the existing
> > MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
> > one - which you or another thread may be using - and clobbers it with
> > the new one.
> 
> It's longer than MAP_FIXED_WEAK :-P
> 
> You'd have to be pretty darn strong to clobber an existing mapping.

I think we're thinking about this all wrong.  We shouldn't document it as
"This is a variant of MAP_FIXED".  We should document it as "Here's an
alternative to MAP_FIXED".

So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
we could add a new paragraph saying "at most one of MAP_FIXED or
MAP_REQUIRED" and "any of the following values".

Now, we should implement MAP_REQUIRED as having each architecture
define _MAP_NOT_A_HINT, and then #define MAP_REQUIRED (MAP_FIXED |
_MAP_NOT_A_HINT), but that's not information to confuse users with.

Also, that lets us add a third option at some point that is Yet Another
Way to interpret the 'addr' argument, by having MAP_FIXED clear and
_MAP_NOT_A_HINT set.

I'm not set on MAP_REQUIRED.  I came up with some awful names
(MAP_TODDLER, MAP_TANTRUM, MAP_ULTIMATUM, MAP_BOSS, MAP_PROGRAM_MANAGER,
etc).  But I think we should drop FIXED from the middle of the name.

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Matthew Wilcox]

On Tue, Dec 05, 2017 at 08:54:35PM -0800, Matthew Wilcox wrote:
> On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
> > Cyril Hrubis  writes:
> > 
> > > Hi!
> > >> > MAP_FIXED_UNIQUE
> > >> > MAP_FIXED_ONCE
> > >> > MAP_FIXED_FRESH
> > >> 
> > >> Well, I can open a poll for the best name, but none of those you are
> > >> proposing sound much better to me. Yeah, naming sucks...
> > >
> > > Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
> > > would probably be a best fit.
> > 
> > Yeah that could work.
> > 
> > I prefer "no clobber" as I just suggested, because the existing
> > MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
> > one - which you or another thread may be using - and clobbers it with
> > the new one.
> 
> It's longer than MAP_FIXED_WEAK :-P
> 
> You'd have to be pretty darn strong to clobber an existing mapping.

I think we're thinking about this all wrong.  We shouldn't document it as
"This is a variant of MAP_FIXED".  We should document it as "Here's an
alternative to MAP_FIXED".

So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
we could add a new paragraph saying "at most one of MAP_FIXED or
MAP_REQUIRED" and "any of the following values".

Now, we should implement MAP_REQUIRED as having each architecture
define _MAP_NOT_A_HINT, and then #define MAP_REQUIRED (MAP_FIXED |
_MAP_NOT_A_HINT), but that's not information to confuse users with.

Also, that lets us add a third option at some point that is Yet Another
Way to interpret the 'addr' argument, by having MAP_FIXED clear and
_MAP_NOT_A_HINT set.

I'm not set on MAP_REQUIRED.  I came up with some awful names
(MAP_TODDLER, MAP_TANTRUM, MAP_ULTIMATUM, MAP_BOSS, MAP_PROGRAM_MANAGER,
etc).  But I think we should drop FIXED from the middle of the name.

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Matthew Wilcox]

On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
> Cyril Hrubis  writes:
> 
> > Hi!
> >> > MAP_FIXED_UNIQUE
> >> > MAP_FIXED_ONCE
> >> > MAP_FIXED_FRESH
> >> 
> >> Well, I can open a poll for the best name, but none of those you are
> >> proposing sound much better to me. Yeah, naming sucks...
> >
> > Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
> > would probably be a best fit.
> 
> Yeah that could work.
> 
> I prefer "no clobber" as I just suggested, because the existing
> MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
> one - which you or another thread may be using - and clobbers it with
> the new one.

It's longer than MAP_FIXED_WEAK :-P

You'd have to be pretty darn strong to clobber an existing mapping.

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Matthew Wilcox]

On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
> Cyril Hrubis  writes:
> 
> > Hi!
> >> > MAP_FIXED_UNIQUE
> >> > MAP_FIXED_ONCE
> >> > MAP_FIXED_FRESH
> >> 
> >> Well, I can open a poll for the best name, but none of those you are
> >> proposing sound much better to me. Yeah, naming sucks...
> >
> > Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
> > would probably be a best fit.
> 
> Yeah that could work.
> 
> I prefer "no clobber" as I just suggested, because the existing
> MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
> one - which you or another thread may be using - and clobbers it with
> the new one.

It's longer than MAP_FIXED_WEAK :-P

You'd have to be pretty darn strong to clobber an existing mapping.

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michael Ellerman]

Cyril Hrubis  writes:

> Hi!
>> > MAP_FIXED_UNIQUE
>> > MAP_FIXED_ONCE
>> > MAP_FIXED_FRESH
>> 
>> Well, I can open a poll for the best name, but none of those you are
>> proposing sound much better to me. Yeah, naming sucks...
>
> Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
> would probably be a best fit.

Yeah that could work.

I prefer "no clobber" as I just suggested, because the existing
MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
one - which you or another thread may be using - and clobbers it with
the new one.

cheers

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michael Ellerman]

Cyril Hrubis  writes:

> Hi!
>> > MAP_FIXED_UNIQUE
>> > MAP_FIXED_ONCE
>> > MAP_FIXED_FRESH
>> 
>> Well, I can open a poll for the best name, but none of those you are
>> proposing sound much better to me. Yeah, naming sucks...
>
> Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
> would probably be a best fit.

Yeah that could work.

I prefer "no clobber" as I just suggested, because the existing
MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
one - which you or another thread may be using - and clobbers it with
the new one.

cheers

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michael Ellerman]

Michal Hocko  writes:

> On Wed 29-11-17 14:25:36, Kees Cook wrote:
>> On Wed, Nov 29, 2017 at 6:42 AM, Michal Hocko  wrote:
>> > The first patch introduced MAP_FIXED_SAFE which enforces the given
>> > address but unlike MAP_FIXED it fails with ENOMEM if the given range
>> > conflicts with an existing one. The flag is introduced as a completely
>> 
>> I still think this name should be better. "SAFE" doesn't say what it's
>> safe from...

Yes exactly.

> It is safe in a sense it doesn't perform any address space dangerous
> operations. mmap is _inherently_ about the address space so the context
> should be kind of clear.

So now you have to define what "dangerous" means.

>> MAP_FIXED_UNIQUE
>> MAP_FIXED_ONCE
>> MAP_FIXED_FRESH
>
> Well, I can open a poll for the best name, but none of those you are
> proposing sound much better to me. Yeah, naming sucks...

I think Kees and I both previously suggested MAP_NO_CLOBBER for the
modifier.

So the obvious option for this would be MAP_FIXED_NO_CLOBBER.

Which is a bit longer sure, but says more or less exactly what it does.

cheers

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michael Ellerman]

Michal Hocko  writes:

> On Wed 29-11-17 14:25:36, Kees Cook wrote:
>> On Wed, Nov 29, 2017 at 6:42 AM, Michal Hocko  wrote:
>> > The first patch introduced MAP_FIXED_SAFE which enforces the given
>> > address but unlike MAP_FIXED it fails with ENOMEM if the given range
>> > conflicts with an existing one. The flag is introduced as a completely
>> 
>> I still think this name should be better. "SAFE" doesn't say what it's
>> safe from...

Yes exactly.

> It is safe in a sense it doesn't perform any address space dangerous
> operations. mmap is _inherently_ about the address space so the context
> should be kind of clear.

So now you have to define what "dangerous" means.

>> MAP_FIXED_UNIQUE
>> MAP_FIXED_ONCE
>> MAP_FIXED_FRESH
>
> Well, I can open a poll for the best name, but none of those you are
> proposing sound much better to me. Yeah, naming sucks...

I think Kees and I both previously suggested MAP_NO_CLOBBER for the
modifier.

So the obvious option for this would be MAP_FIXED_NO_CLOBBER.

Which is a bit longer sure, but says more or less exactly what it does.

cheers

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Cyril Hrubis]

Hi!
> > MAP_FIXED_UNIQUE
> > MAP_FIXED_ONCE
> > MAP_FIXED_FRESH
> 
> Well, I can open a poll for the best name, but none of those you are
> proposing sound much better to me. Yeah, naming sucks...

Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
would probably be a best fit.

-- 
Cyril Hrubis
chru...@suse.cz

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Cyril Hrubis]

Hi!
> > MAP_FIXED_UNIQUE
> > MAP_FIXED_ONCE
> > MAP_FIXED_FRESH
> 
> Well, I can open a poll for the best name, but none of those you are
> proposing sound much better to me. Yeah, naming sucks...

Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
would probably be a best fit.

-- 
Cyril Hrubis
chru...@suse.cz

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

On Wed 29-11-17 14:25:36, Kees Cook wrote:
> On Wed, Nov 29, 2017 at 6:42 AM, Michal Hocko  wrote:
> > The first patch introduced MAP_FIXED_SAFE which enforces the given
> > address but unlike MAP_FIXED it fails with ENOMEM if the given range
> > conflicts with an existing one. The flag is introduced as a completely
> 
> I still think this name should be better. "SAFE" doesn't say what it's
> safe from...

It is safe in a sense it doesn't perform any address space dangerous
operations. mmap is _inherently_ about the address space so the context
should be kind of clear.

> MAP_FIXED_UNIQUE
> MAP_FIXED_ONCE
> MAP_FIXED_FRESH

Well, I can open a poll for the best name, but none of those you are
proposing sound much better to me. Yeah, naming sucks...
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

On Wed 29-11-17 14:25:36, Kees Cook wrote:
> On Wed, Nov 29, 2017 at 6:42 AM, Michal Hocko  wrote:
> > The first patch introduced MAP_FIXED_SAFE which enforces the given
> > address but unlike MAP_FIXED it fails with ENOMEM if the given range
> > conflicts with an existing one. The flag is introduced as a completely
> 
> I still think this name should be better. "SAFE" doesn't say what it's
> safe from...

It is safe in a sense it doesn't perform any address space dangerous
operations. mmap is _inherently_ about the address space so the context
should be kind of clear.

> MAP_FIXED_UNIQUE
> MAP_FIXED_ONCE
> MAP_FIXED_FRESH

Well, I can open a poll for the best name, but none of those you are
proposing sound much better to me. Yeah, naming sucks...
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Kees Cook]

On Wed, Nov 29, 2017 at 6:42 AM, Michal Hocko  wrote:
> The first patch introduced MAP_FIXED_SAFE which enforces the given
> address but unlike MAP_FIXED it fails with ENOMEM if the given range
> conflicts with an existing one. The flag is introduced as a completely

I still think this name should be better. "SAFE" doesn't say what it's
safe from...

MAP_FIXED_UNIQUE
MAP_FIXED_ONCE
MAP_FIXED_FRESH

?

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Kees Cook]

On Wed, Nov 29, 2017 at 6:42 AM, Michal Hocko  wrote:
> The first patch introduced MAP_FIXED_SAFE which enforces the given
> address but unlike MAP_FIXED it fails with ENOMEM if the given range
> conflicts with an existing one. The flag is introduced as a completely

I still think this name should be better. "SAFE" doesn't say what it's
safe from...

MAP_FIXED_UNIQUE
MAP_FIXED_ONCE
MAP_FIXED_FRESH

?

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Kees Cook]

On Wed, Nov 29, 2017 at 7:13 AM, Rasmus Villemoes
 wrote:
> On 2017-11-29 15:42, Michal Hocko wrote:
>>
>> The first patch introduced MAP_FIXED_SAFE which enforces the given
>> address but unlike MAP_FIXED it fails with ENOMEM if the given range
>> conflicts with an existing one.
>
> [s/ENOMEM/EEXIST/, as it seems you also did in the actual patch and
> changelog]
>
>>The flag is introduced as a completely
>> new one rather than a MAP_FIXED extension because of the backward
>> compatibility. We really want a never-clobber semantic even on older
>> kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
>> flags evaluation because we do not EINVAL on unknown flags. On those
>> kernels we would simply use the traditional hint based semantic so the
>> caller can still get a different address (which sucks) but at least not
>> silently corrupt an existing mapping. I do not see a good way around
>> that.
>
> I think it would be nice if this rationale was in the 1/2 changelog,
> along with the hint about what userspace that wants to be compatible
> with old kernels will have to do (namely, check that it got what it
> requested) - which I see you did put in the man page.

Okay, so ignore my other email, I must have misunderstood. It _is_,
quite intentionally, being exposed to userspace. Cool by me. :)

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Kees Cook]

On Wed, Nov 29, 2017 at 7:13 AM, Rasmus Villemoes
 wrote:
> On 2017-11-29 15:42, Michal Hocko wrote:
>>
>> The first patch introduced MAP_FIXED_SAFE which enforces the given
>> address but unlike MAP_FIXED it fails with ENOMEM if the given range
>> conflicts with an existing one.
>
> [s/ENOMEM/EEXIST/, as it seems you also did in the actual patch and
> changelog]
>
>>The flag is introduced as a completely
>> new one rather than a MAP_FIXED extension because of the backward
>> compatibility. We really want a never-clobber semantic even on older
>> kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
>> flags evaluation because we do not EINVAL on unknown flags. On those
>> kernels we would simply use the traditional hint based semantic so the
>> caller can still get a different address (which sucks) but at least not
>> silently corrupt an existing mapping. I do not see a good way around
>> that.
>
> I think it would be nice if this rationale was in the 1/2 changelog,
> along with the hint about what userspace that wants to be compatible
> with old kernels will have to do (namely, check that it got what it
> requested) - which I see you did put in the man page.

Okay, so ignore my other email, I must have misunderstood. It _is_,
quite intentionally, being exposed to userspace. Cool by me. :)

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Kees Cook]

On Wed, Nov 29, 2017 at 6:42 AM, Michal Hocko  wrote:
> Except we won't export expose the new semantic to the userspace at all.

I'm confused: the changes in patch 1 are explicitly adding
MAP_FIXED_SAFE to the uapi. If it's not supposed to be exposed,
shouldn't it go somewhere else?

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Kees Cook]

On Wed, Nov 29, 2017 at 6:42 AM, Michal Hocko  wrote:
> Except we won't export expose the new semantic to the userspace at all.

I'm confused: the changes in patch 1 are explicitly adding
MAP_FIXED_SAFE to the uapi. If it's not supposed to be exposed,
shouldn't it go somewhere else?

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

On Wed 29-11-17 16:13:53, Rasmus Villemoes wrote:
> On 2017-11-29 15:42, Michal Hocko wrote:
[...]
> >The flag is introduced as a completely
> > new one rather than a MAP_FIXED extension because of the backward
> > compatibility. We really want a never-clobber semantic even on older
> > kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
> > flags evaluation because we do not EINVAL on unknown flags. On those
> > kernels we would simply use the traditional hint based semantic so the
> > caller can still get a different address (which sucks) but at least not
> > silently corrupt an existing mapping. I do not see a good way around
> > that.
> 
> I think it would be nice if this rationale was in the 1/2 changelog,
> along with the hint about what userspace that wants to be compatible
> with old kernels will have to do (namely, check that it got what it
> requested) - which I see you did put in the man page.

OK, I've added there.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

On Wed 29-11-17 16:13:53, Rasmus Villemoes wrote:
> On 2017-11-29 15:42, Michal Hocko wrote:
[...]
> >The flag is introduced as a completely
> > new one rather than a MAP_FIXED extension because of the backward
> > compatibility. We really want a never-clobber semantic even on older
> > kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
> > flags evaluation because we do not EINVAL on unknown flags. On those
> > kernels we would simply use the traditional hint based semantic so the
> > caller can still get a different address (which sucks) but at least not
> > silently corrupt an existing mapping. I do not see a good way around
> > that.
> 
> I think it would be nice if this rationale was in the 1/2 changelog,
> along with the hint about what userspace that wants to be compatible
> with old kernels will have to do (namely, check that it got what it
> requested) - which I see you did put in the man page.

OK, I've added there.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Rasmus Villemoes]

On 2017-11-29 15:42, Michal Hocko wrote:
> 
> The first patch introduced MAP_FIXED_SAFE which enforces the given
> address but unlike MAP_FIXED it fails with ENOMEM if the given range
> conflicts with an existing one.

[s/ENOMEM/EEXIST/, as it seems you also did in the actual patch and
changelog]

>The flag is introduced as a completely
> new one rather than a MAP_FIXED extension because of the backward
> compatibility. We really want a never-clobber semantic even on older
> kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
> flags evaluation because we do not EINVAL on unknown flags. On those
> kernels we would simply use the traditional hint based semantic so the
> caller can still get a different address (which sucks) but at least not
> silently corrupt an existing mapping. I do not see a good way around
> that.

I think it would be nice if this rationale was in the 1/2 changelog,
along with the hint about what userspace that wants to be compatible
with old kernels will have to do (namely, check that it got what it
requested) - which I see you did put in the man page.

-- 
Rasmus Villemoes
Software Developer
Prevas A/S
Hedeager 3
DK-8200 Aarhus N
+45 51210274
rasmus.villem...@prevas.dk
www.prevas.dk

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Rasmus Villemoes]

On 2017-11-29 15:42, Michal Hocko wrote:
> 
> The first patch introduced MAP_FIXED_SAFE which enforces the given
> address but unlike MAP_FIXED it fails with ENOMEM if the given range
> conflicts with an existing one.

[s/ENOMEM/EEXIST/, as it seems you also did in the actual patch and
changelog]

>The flag is introduced as a completely
> new one rather than a MAP_FIXED extension because of the backward
> compatibility. We really want a never-clobber semantic even on older
> kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
> flags evaluation because we do not EINVAL on unknown flags. On those
> kernels we would simply use the traditional hint based semantic so the
> caller can still get a different address (which sucks) but at least not
> silently corrupt an existing mapping. I do not see a good way around
> that.

I think it would be nice if this rationale was in the 1/2 changelog,
along with the hint about what userspace that wants to be compatible
with old kernels will have to do (namely, check that it got what it
requested) - which I see you did put in the man page.

-- 
Rasmus Villemoes
Software Developer
Prevas A/S
Hedeager 3
DK-8200 Aarhus N
+45 51210274
rasmus.villem...@prevas.dk
www.prevas.dk

[PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

Hi,
I am resending with RFC dropped and ask for inclusion. There haven't
been any fundamental objections for the RFC [1]. I have also prepared
a man page patch which is 0/3 of this series.

This has started as a follow up discussion [2][3] resulting in the
runtime failure caused by hardening patch [4] which removes MAP_FIXED
from the elf loader because MAP_FIXED is inherently dangerous as it
might silently clobber an existing underlying mapping (e.g. stack). The
reason for the failure is that some architectures enforce an alignment
for the given address hint without MAP_FIXED used (e.g. for shared or
file backed mappings).

One way around this would be excluding those archs which do alignment
tricks from the hardening [5]. The patch is really trivial but it has
been objected, rightfully so, that this screams for a more generic
solution. We basically want a non-destructive MAP_FIXED.

The first patch introduced MAP_FIXED_SAFE which enforces the given
address but unlike MAP_FIXED it fails with ENOMEM if the given range
conflicts with an existing one. The flag is introduced as a completely
new one rather than a MAP_FIXED extension because of the backward
compatibility. We really want a never-clobber semantic even on older
kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
flags evaluation because we do not EINVAL on unknown flags. On those
kernels we would simply use the traditional hint based semantic so the
caller can still get a different address (which sucks) but at least not
silently corrupt an existing mapping. I do not see a good way around
that. Except we won't export expose the new semantic to the userspace at
all. 

It seems there are users who would like to have something like that.
Jemalloc has been mentioned by Michael Ellerman [6]

Florian Weimer has mentioned the following:
: glibc ld.so currently maps DSOs without hints.  This means that the kernel
: will map right next to each other, and the offsets between them a completely
: predictable.  We would like to change that and supply a random address in a
: window of the address space.  If there is a conflict, we do not want the
: kernel to pick a non-random address. Instead, we would try again with a
: random address.

John Hubbard has mentioned CUDA example
: a) Searches /proc//maps for a "suitable" region of available
: VA space.  "Suitable" generally means it has to have a base address
: within a certain limited range (a particular device model might
: have odd limitations, for example), it has to be large enough, and
: alignment has to be large enough (again, various devices may have
: constraints that lead us to do this).
: 
: This is of course subject to races with other threads in the process.
: 
: Let's say it finds a region starting at va.
: 
: b) Next it does: 
: p = mmap(va, ...) 
: 
: *without* setting MAP_FIXED, of course (so va is just a hint), to
: attempt to safely reserve that region. If p != va, then in most cases,
: this is a failure (almost certainly due to another thread getting a
: mapping from that region before we did), and so this layer now has to
: call munmap(), before returning a "failure: retry" to upper layers.
: 
: IMPROVEMENT: --> if instead, we could call this:
: 
: p = mmap(va, ... MAP_FIXED_SAFE ...)
: 
: , then we could skip the munmap() call upon failure. This
: is a small thing, but it is useful here. (Thanks to Piotr
: Jaroszynski and Mark Hairgrove for helping me get that detail
: exactly right, btw.)
: 
: c) After that, CUDA suballocates from p, via: 
:  
:  q = mmap(sub_region_start, ... MAP_FIXED ...)
: 
: Interestingly enough, "freeing" is also done via MAP_FIXED, and
: setting PROT_NONE to the subregion. Anyway, I just included (c) for
: general interest.

Atomic address range probing in the multithreaded programs in general
sounds like an interesting thing to me.

The second patch simply replaces MAP_FIXED use in elf loader by
MAP_FIXED_SAFE. I believe other places which rely on MAP_FIXED should
follow. Actually real MAP_FIXED usages should be docummented properly
and they should be more of an exception.

Does anybody see any fundamental reasons why this is a wrong approach?

Diffstat says
 arch/alpha/include/uapi/asm/mman.h   |  2 ++
 arch/metag/kernel/process.c  |  6 +-
 arch/mips/include/uapi/asm/mman.h|  2 ++
 arch/parisc/include/uapi/asm/mman.h  |  2 ++
 arch/powerpc/include/uapi/asm/mman.h |  1 +
 arch/sparc/include/uapi/asm/mman.h   |  1 +
 arch/tile/include/uapi/asm/mman.h|  1 +
 arch/xtensa/include/uapi/asm/mman.h  |  2 ++
 fs/binfmt_elf.c  | 12 
 include/uapi/asm-generic/mman.h  |  1 +
 mm/mmap.c| 11 +++
 11 files changed, 36 insertions(+), 5 deletions(-)

[1] http://lkml.kernel.org/r/20171116101900.13621-1-mho...@kernel.org
[2] http://lkml.kernel.org/r/20171107162217.382cd...@canb.auug.org.au
[3] 

[PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

Hi,
I am resending with RFC dropped and ask for inclusion. There haven't
been any fundamental objections for the RFC [1]. I have also prepared
a man page patch which is 0/3 of this series.

This has started as a follow up discussion [2][3] resulting in the
runtime failure caused by hardening patch [4] which removes MAP_FIXED
from the elf loader because MAP_FIXED is inherently dangerous as it
might silently clobber an existing underlying mapping (e.g. stack). The
reason for the failure is that some architectures enforce an alignment
for the given address hint without MAP_FIXED used (e.g. for shared or
file backed mappings).

One way around this would be excluding those archs which do alignment
tricks from the hardening [5]. The patch is really trivial but it has
been objected, rightfully so, that this screams for a more generic
solution. We basically want a non-destructive MAP_FIXED.

The first patch introduced MAP_FIXED_SAFE which enforces the given
address but unlike MAP_FIXED it fails with ENOMEM if the given range
conflicts with an existing one. The flag is introduced as a completely
new one rather than a MAP_FIXED extension because of the backward
compatibility. We really want a never-clobber semantic even on older
kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
flags evaluation because we do not EINVAL on unknown flags. On those
kernels we would simply use the traditional hint based semantic so the
caller can still get a different address (which sucks) but at least not
silently corrupt an existing mapping. I do not see a good way around
that. Except we won't export expose the new semantic to the userspace at
all. 

It seems there are users who would like to have something like that.
Jemalloc has been mentioned by Michael Ellerman [6]

Florian Weimer has mentioned the following:
: glibc ld.so currently maps DSOs without hints.  This means that the kernel
: will map right next to each other, and the offsets between them a completely
: predictable.  We would like to change that and supply a random address in a
: window of the address space.  If there is a conflict, we do not want the
: kernel to pick a non-random address. Instead, we would try again with a
: random address.

John Hubbard has mentioned CUDA example
: a) Searches /proc//maps for a "suitable" region of available
: VA space.  "Suitable" generally means it has to have a base address
: within a certain limited range (a particular device model might
: have odd limitations, for example), it has to be large enough, and
: alignment has to be large enough (again, various devices may have
: constraints that lead us to do this).
: 
: This is of course subject to races with other threads in the process.
: 
: Let's say it finds a region starting at va.
: 
: b) Next it does: 
: p = mmap(va, ...) 
: 
: *without* setting MAP_FIXED, of course (so va is just a hint), to
: attempt to safely reserve that region. If p != va, then in most cases,
: this is a failure (almost certainly due to another thread getting a
: mapping from that region before we did), and so this layer now has to
: call munmap(), before returning a "failure: retry" to upper layers.
: 
: IMPROVEMENT: --> if instead, we could call this:
: 
: p = mmap(va, ... MAP_FIXED_SAFE ...)
: 
: , then we could skip the munmap() call upon failure. This
: is a small thing, but it is useful here. (Thanks to Piotr
: Jaroszynski and Mark Hairgrove for helping me get that detail
: exactly right, btw.)
: 
: c) After that, CUDA suballocates from p, via: 
:  
:  q = mmap(sub_region_start, ... MAP_FIXED ...)
: 
: Interestingly enough, "freeing" is also done via MAP_FIXED, and
: setting PROT_NONE to the subregion. Anyway, I just included (c) for
: general interest.

Atomic address range probing in the multithreaded programs in general
sounds like an interesting thing to me.

The second patch simply replaces MAP_FIXED use in elf loader by
MAP_FIXED_SAFE. I believe other places which rely on MAP_FIXED should
follow. Actually real MAP_FIXED usages should be docummented properly
and they should be more of an exception.

Does anybody see any fundamental reasons why this is a wrong approach?

Diffstat says
 arch/alpha/include/uapi/asm/mman.h   |  2 ++
 arch/metag/kernel/process.c  |  6 +-
 arch/mips/include/uapi/asm/mman.h|  2 ++
 arch/parisc/include/uapi/asm/mman.h  |  2 ++
 arch/powerpc/include/uapi/asm/mman.h |  1 +
 arch/sparc/include/uapi/asm/mman.h   |  1 +
 arch/tile/include/uapi/asm/mman.h|  1 +
 arch/xtensa/include/uapi/asm/mman.h  |  2 ++
 fs/binfmt_elf.c  | 12 
 include/uapi/asm-generic/mman.h  |  1 +
 mm/mmap.c| 11 +++
 11 files changed, 36 insertions(+), 5 deletions(-)

[1] http://lkml.kernel.org/r/20171116101900.13621-1-mho...@kernel.org
[2] http://lkml.kernel.org/r/20171107162217.382cd...@canb.auug.org.au
[3] 

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Khalid Aziz]

On 11/24/2017 01:54 AM, Michal Hocko wrote:

Are there any more concerns? So far the biggest one was the name. The
other which suggests a flag as a modifier has been sorted out hopefully.
Is there anymore more before we can consider this for merging? Well
except for man page update which I will prepare of course. Can we target
this to 4.16?


I do not have concerns with this approach. I prefer a new flag as 
opposed to a modifier and the name is ok by me.


Reviewed-by: Khalid Aziz 

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Khalid Aziz]

On 11/24/2017 01:54 AM, Michal Hocko wrote:

Are there any more concerns? So far the biggest one was the name. The
other which suggests a flag as a modifier has been sorted out hopefully.
Is there anymore more before we can consider this for merging? Well
except for man page update which I will prepare of course. Can we target
this to 4.16?


I do not have concerns with this approach. I prefer a new flag as 
opposed to a modifier and the name is ok by me.


Reviewed-by: Khalid Aziz 

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

Are there any more concerns? So far the biggest one was the name. The
other which suggests a flag as a modifier has been sorted out hopefully.
Is there anymore more before we can consider this for merging? Well
except for man page update which I will prepare of course. Can we target
this to 4.16?

On Thu 16-11-17 13:14:38, Michal Hocko wrote:
> [Ups, managed to screw the subject - fix it]
> 
> On Thu 16-11-17 11:18:58, Michal Hocko wrote:
> > Hi,
> > this has started as a follow up discussion [1][2] resulting in the
> > runtime failure caused by hardening patch [3] which removes MAP_FIXED
> > from the elf loader because MAP_FIXED is inherently dangerous as it
> > might silently clobber and existing underlying mapping (e.g. stack). The
> > reason for the failure is that some architectures enforce an alignment
> > for the given address hint without MAP_FIXED used (e.g. for shared or
> > file backed mappings).
> > 
> > One way around this would be excluding those archs which do alignment
> > tricks from the hardening [4]. The patch is really trivial but it has
> > been objected, rightfully so, that this screams for a more generic
> > solution. We basically want a non-destructive MAP_FIXED.
> > 
> > The first patch introduced MAP_FIXED_SAFE which enforces the given
> > address but unlike MAP_FIXED it fails with ENOMEM if the given range
> > conflicts with an existing one. The flag is introduced as a completely
> > new flag rather than a MAP_FIXED extension because of the backward
> > compatibility. We really want a never-clobber semantic even on older
> > kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
> > flags evaluation because we do not EINVAL on unknown flags. On those
> > kernels we would simply use the traditional hint based semantic so the
> > caller can still get a different address (which sucks) but at least not
> > silently corrupt an existing mapping. I do not see a good way around
> > that. Except we won't export expose the new semantic to the userspace at
> > all. It seems there are users who would like to have something like that
> > [5], though. Atomic address range probing in the multithreaded programs
> > sounds like an interesting thing to me as well, although I do not have
> > any specific usecase in mind.
> > 
> > The second patch simply replaces MAP_FIXED use in elf loader by
> > MAP_FIXED_SAFE. I believe other places which rely on MAP_FIXED should
> > follow. Actually real MAP_FIXED usages should be docummented properly
> > and they should be more of an exception.
> > 
> > Does anybody see any fundamental reasons why this is a wrong approach?
> > 
> > Diffstat says
> >  arch/alpha/include/uapi/asm/mman.h   |  2 ++
> >  arch/metag/kernel/process.c  |  6 +-
> >  arch/mips/include/uapi/asm/mman.h|  2 ++
> >  arch/parisc/include/uapi/asm/mman.h  |  2 ++
> >  arch/powerpc/include/uapi/asm/mman.h |  1 +
> >  arch/sparc/include/uapi/asm/mman.h   |  1 +
> >  arch/tile/include/uapi/asm/mman.h|  1 +
> >  arch/xtensa/include/uapi/asm/mman.h  |  2 ++
> >  fs/binfmt_elf.c  | 12 
> >  include/uapi/asm-generic/mman.h  |  1 +
> >  mm/mmap.c| 11 +++
> >  11 files changed, 36 insertions(+), 5 deletions(-)
> > 
> > [1] http://lkml.kernel.org/r/20171107162217.382cd...@canb.auug.org.au
> > [2] http://lkml.kernel.org/r/1510048229.12079.7.ca...@abdul.in.ibm.com
> > [3] http://lkml.kernel.org/r/20171023082608.6167-1-mho...@kernel.org
> > [4] http://lkml.kernel.org/r/20171113094203.aofz2e7kueitk...@dhcp22.suse.cz
> > [5] http://lkml.kernel.org/r/87efp1w7vy@concordia.ellerman.id.au
> > 
> > 
> > --
> > To unsubscribe, send a message with 'unsubscribe linux-mm' in
> > the body to majord...@kvack.org.  For more info on Linux MM,
> > see: http://www.linux-mm.org/ .
> > Don't email: mailto:"d...@kvack.org;> em...@kvack.org 
> 
> -- 
> Michal Hocko
> SUSE Labs

-- 
Michal Hocko
SUSE Labs

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

Are there any more concerns? So far the biggest one was the name. The
other which suggests a flag as a modifier has been sorted out hopefully.
Is there anymore more before we can consider this for merging? Well
except for man page update which I will prepare of course. Can we target
this to 4.16?

On Thu 16-11-17 13:14:38, Michal Hocko wrote:
> [Ups, managed to screw the subject - fix it]
> 
> On Thu 16-11-17 11:18:58, Michal Hocko wrote:
> > Hi,
> > this has started as a follow up discussion [1][2] resulting in the
> > runtime failure caused by hardening patch [3] which removes MAP_FIXED
> > from the elf loader because MAP_FIXED is inherently dangerous as it
> > might silently clobber and existing underlying mapping (e.g. stack). The
> > reason for the failure is that some architectures enforce an alignment
> > for the given address hint without MAP_FIXED used (e.g. for shared or
> > file backed mappings).
> > 
> > One way around this would be excluding those archs which do alignment
> > tricks from the hardening [4]. The patch is really trivial but it has
> > been objected, rightfully so, that this screams for a more generic
> > solution. We basically want a non-destructive MAP_FIXED.
> > 
> > The first patch introduced MAP_FIXED_SAFE which enforces the given
> > address but unlike MAP_FIXED it fails with ENOMEM if the given range
> > conflicts with an existing one. The flag is introduced as a completely
> > new flag rather than a MAP_FIXED extension because of the backward
> > compatibility. We really want a never-clobber semantic even on older
> > kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
> > flags evaluation because we do not EINVAL on unknown flags. On those
> > kernels we would simply use the traditional hint based semantic so the
> > caller can still get a different address (which sucks) but at least not
> > silently corrupt an existing mapping. I do not see a good way around
> > that. Except we won't export expose the new semantic to the userspace at
> > all. It seems there are users who would like to have something like that
> > [5], though. Atomic address range probing in the multithreaded programs
> > sounds like an interesting thing to me as well, although I do not have
> > any specific usecase in mind.
> > 
> > The second patch simply replaces MAP_FIXED use in elf loader by
> > MAP_FIXED_SAFE. I believe other places which rely on MAP_FIXED should
> > follow. Actually real MAP_FIXED usages should be docummented properly
> > and they should be more of an exception.
> > 
> > Does anybody see any fundamental reasons why this is a wrong approach?
> > 
> > Diffstat says
> >  arch/alpha/include/uapi/asm/mman.h   |  2 ++
> >  arch/metag/kernel/process.c  |  6 +-
> >  arch/mips/include/uapi/asm/mman.h|  2 ++
> >  arch/parisc/include/uapi/asm/mman.h  |  2 ++
> >  arch/powerpc/include/uapi/asm/mman.h |  1 +
> >  arch/sparc/include/uapi/asm/mman.h   |  1 +
> >  arch/tile/include/uapi/asm/mman.h|  1 +
> >  arch/xtensa/include/uapi/asm/mman.h  |  2 ++
> >  fs/binfmt_elf.c  | 12 
> >  include/uapi/asm-generic/mman.h  |  1 +
> >  mm/mmap.c| 11 +++
> >  11 files changed, 36 insertions(+), 5 deletions(-)
> > 
> > [1] http://lkml.kernel.org/r/20171107162217.382cd...@canb.auug.org.au
> > [2] http://lkml.kernel.org/r/1510048229.12079.7.ca...@abdul.in.ibm.com
> > [3] http://lkml.kernel.org/r/20171023082608.6167-1-mho...@kernel.org
> > [4] http://lkml.kernel.org/r/20171113094203.aofz2e7kueitk...@dhcp22.suse.cz
> > [5] http://lkml.kernel.org/r/87efp1w7vy@concordia.ellerman.id.au
> > 
> > 
> > --
> > To unsubscribe, send a message with 'unsubscribe linux-mm' in
> > the body to majord...@kvack.org.  For more info on Linux MM,
> > see: http://www.linux-mm.org/ .
> > Don't email: mailto:"d...@kvack.org;> em...@kvack.org 
> 
> -- 
> Michal Hocko
> SUSE Labs

-- 
Michal Hocko
SUSE Labs

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Vlastimil Babka]

On 11/22/2017 02:12 PM, Michal Hocko wrote:
> I will be probably stubborn and go with a shorter name I have currently.
> I am not very fond-of-very-long-names.

The short synonym for the last word is "German"

SCNR :P

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Vlastimil Babka]

On 11/22/2017 02:12 PM, Michal Hocko wrote:
> I will be probably stubborn and go with a shorter name I have currently.
> I am not very fond-of-very-long-names.

The short synonym for the last word is "German"

SCNR :P

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

On Tue 21-11-17 17:48:31, John Hubbard wrote:
[...]
> Hi Michal,
> 
> Yes, it really is useful for user space. I'll use CUDA as an example, but I 
> think anything that enforces a uniform virtual addressing scheme across CPUs
> and devices, probably has to do something eerily similar. CUDA does this:
> 
> a) Searches /proc//maps for a "suitable" region of available VA space. 
> "Suitable" generally means it has to have a base address within a certain
> limited range (a particular device model might have odd limitations, for 
> example), it has to be large enough, and alignment has to be large enough
> (again, various devices may have constraints that lead us to do this).
> 
> This is of course subject to races with other threads in the process.
> 
> Let's say it finds a region starting at va.
> 
> b) Next it does: 
> p = mmap(va, ...) 
> 
> *without* setting MAP_FIXED, of course (so va is just a hint), to attempt to
> safely reserve that region. If p != va, then in most cases, this is a failure
> (almost certainly due to another thread getting a mapping from that region
> before we did), and so this layer now has to call munmap(), before returning
> a "failure: retry" to upper layers.
> 
> IMPROVEMENT: --> if instead, we could call this:
> 
> p = mmap(va, ... MAP_FIXED_NO_CLOBBER ...)
> 
> , then we could skip the munmap() call upon failure. This is a small 
> thing, 
> but it is useful here. (Thanks to Piotr Jaroszynski and Mark Hairgrove
> for helping me get that detail exactly right, btw.)
> 
> c) After that, CUDA suballocates from p, via: 
>  
>  q = mmap(sub_region_start, ... MAP_FIXED ...)
> 
> Interestingly enough, "freeing" is also done via MAP_FIXED, and setting 
> PROT_NONE
> to the subregion. Anyway, I just included (c) for general interest.

OK, I will add this to the changelog. This is basically the "Atomic
address range probing in the multithreaded programs" I've had in the
cover letter.

> I expect that as we continue working on the open source compute software 
> stack,
> this new capability will be useful there, too.
> 
> Oh, and on the naming, when I described how your implementation worked 
> (without
> naming it) to Piotr, he said, "oh, something like map-fixed-no-clobber?". So I
> think my miniature sociology naming data point here can bolster the case ever 
> so
> slightly for calling it MAP_FIXED_NO_CLOBBER. haha. :)

I will be probably stubborn and go with a shorter name I have currently.
I am not very fond-of-very-long-names.
-- 
Michal Hocko
SUSE Labs

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

On Tue 21-11-17 17:48:31, John Hubbard wrote:
[...]
> Hi Michal,
> 
> Yes, it really is useful for user space. I'll use CUDA as an example, but I 
> think anything that enforces a uniform virtual addressing scheme across CPUs
> and devices, probably has to do something eerily similar. CUDA does this:
> 
> a) Searches /proc//maps for a "suitable" region of available VA space. 
> "Suitable" generally means it has to have a base address within a certain
> limited range (a particular device model might have odd limitations, for 
> example), it has to be large enough, and alignment has to be large enough
> (again, various devices may have constraints that lead us to do this).
> 
> This is of course subject to races with other threads in the process.
> 
> Let's say it finds a region starting at va.
> 
> b) Next it does: 
> p = mmap(va, ...) 
> 
> *without* setting MAP_FIXED, of course (so va is just a hint), to attempt to
> safely reserve that region. If p != va, then in most cases, this is a failure
> (almost certainly due to another thread getting a mapping from that region
> before we did), and so this layer now has to call munmap(), before returning
> a "failure: retry" to upper layers.
> 
> IMPROVEMENT: --> if instead, we could call this:
> 
> p = mmap(va, ... MAP_FIXED_NO_CLOBBER ...)
> 
> , then we could skip the munmap() call upon failure. This is a small 
> thing, 
> but it is useful here. (Thanks to Piotr Jaroszynski and Mark Hairgrove
> for helping me get that detail exactly right, btw.)
> 
> c) After that, CUDA suballocates from p, via: 
>  
>  q = mmap(sub_region_start, ... MAP_FIXED ...)
> 
> Interestingly enough, "freeing" is also done via MAP_FIXED, and setting 
> PROT_NONE
> to the subregion. Anyway, I just included (c) for general interest.

OK, I will add this to the changelog. This is basically the "Atomic
address range probing in the multithreaded programs" I've had in the
cover letter.

> I expect that as we continue working on the open source compute software 
> stack,
> this new capability will be useful there, too.
> 
> Oh, and on the naming, when I described how your implementation worked 
> (without
> naming it) to Piotr, he said, "oh, something like map-fixed-no-clobber?". So I
> think my miniature sociology naming data point here can bolster the case ever 
> so
> slightly for calling it MAP_FIXED_NO_CLOBBER. haha. :)

I will be probably stubborn and go with a shorter name I have currently.
I am not very fond-of-very-long-names.
-- 
Michal Hocko
SUSE Labs

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[John Hubbard]

On 11/20/2017 01:05 AM, Michal Hocko wrote:
> On Fri 17-11-17 00:45:49, John Hubbard wrote:
>> On 11/16/2017 04:14 AM, Michal Hocko wrote:
>>> [Ups, managed to screw the subject - fix it]
>>>
>>> On Thu 16-11-17 11:18:58, Michal Hocko wrote:
 Hi,
 this has started as a follow up discussion [1][2] resulting in the
 runtime failure caused by hardening patch [3] which removes MAP_FIXED
 from the elf loader because MAP_FIXED is inherently dangerous as it
 might silently clobber and existing underlying mapping (e.g. stack). The
 reason for the failure is that some architectures enforce an alignment
 for the given address hint without MAP_FIXED used (e.g. for shared or
 file backed mappings).

 One way around this would be excluding those archs which do alignment
 tricks from the hardening [4]. The patch is really trivial but it has
 been objected, rightfully so, that this screams for a more generic
 solution. We basically want a non-destructive MAP_FIXED.

 The first patch introduced MAP_FIXED_SAFE which enforces the given
 address but unlike MAP_FIXED it fails with ENOMEM if the given range
 conflicts with an existing one. The flag is introduced as a completely
 new flag rather than a MAP_FIXED extension because of the backward
 compatibility. We really want a never-clobber semantic even on older
 kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
 flags evaluation because we do not EINVAL on unknown flags. On those
 kernels we would simply use the traditional hint based semantic so the
 caller can still get a different address (which sucks) but at least not
 silently corrupt an existing mapping. I do not see a good way around
 that. Except we won't export expose the new semantic to the userspace at
 all. It seems there are users who would like to have something like that
 [5], though. Atomic address range probing in the multithreaded programs
 sounds like an interesting thing to me as well, although I do not have
 any specific usecase in mind.
>>
>> Hi Michal,
>>
>> From looking at the patchset, it seems to me that the new MAP_FIXED_SAFE
>> (or whatever it ends up being named) *would* be passed through from
>> user space. When you say that "we won't export expose the new semantic 
>> to the userspace at all", do you mean that glibc won't add it? Or
>> is there something I'm missing, that prevents that flag from getting
>> from the syscall, to do_mmap()?
> 
> I meant that I could make it an internal flag outside of the map_type
> space. So the userspace will not be able to use it.
>  
>> On the usage: there are cases in user space that could probably make
>> good use of a no-clobber hint to MAP_FIXED. The user space code
>> that surrounds HMM (speaking loosely there--it's really any user space
>> code that manages a unified memory address space, across devices)
>> often ends up using MAP_FIXED, but MAP_FIXED crams several features
>> into one flag: an exact address, an "atomic" switch to the new mapping,
>> and unmapping the old mappings. That's pretty overloaded, so being
>> able to split it up a bit, by removing one of those features, seems
>> useful.
> 
> Yes, atomic address range probing sounds useful. I cannot comment on HMM
> usage but if you have any more specific I would welcome any links to add
> them to the changelog.
> 

Hi Michal,

Yes, it really is useful for user space. I'll use CUDA as an example, but I 
think anything that enforces a uniform virtual addressing scheme across CPUs
and devices, probably has to do something eerily similar. CUDA does this:

a) Searches /proc//maps for a "suitable" region of available VA space. 
"Suitable" generally means it has to have a base address within a certain
limited range (a particular device model might have odd limitations, for 
example), it has to be large enough, and alignment has to be large enough
(again, various devices may have constraints that lead us to do this).

This is of course subject to races with other threads in the process.

Let's say it finds a region starting at va.

b) Next it does: 
p = mmap(va, ...) 

*without* setting MAP_FIXED, of course (so va is just a hint), to attempt to
safely reserve that region. If p != va, then in most cases, this is a failure
(almost certainly due to another thread getting a mapping from that region
before we did), and so this layer now has to call munmap(), before returning
a "failure: retry" to upper layers.

IMPROVEMENT: --> if instead, we could call this:

p = mmap(va, ... MAP_FIXED_NO_CLOBBER ...)

, then we could skip the munmap() call upon failure. This is a small 
thing, 
but it is useful here. (Thanks to Piotr Jaroszynski and Mark Hairgrove
for helping me get that detail exactly right, btw.)

c) After that, CUDA suballocates from p, via: 
 
 q = mmap(sub_region_start, ... MAP_FIXED ...)

Interestingly enough, 

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[John Hubbard]

On 11/20/2017 01:05 AM, Michal Hocko wrote:
> On Fri 17-11-17 00:45:49, John Hubbard wrote:
>> On 11/16/2017 04:14 AM, Michal Hocko wrote:
>>> [Ups, managed to screw the subject - fix it]
>>>
>>> On Thu 16-11-17 11:18:58, Michal Hocko wrote:
 Hi,
 this has started as a follow up discussion [1][2] resulting in the
 runtime failure caused by hardening patch [3] which removes MAP_FIXED
 from the elf loader because MAP_FIXED is inherently dangerous as it
 might silently clobber and existing underlying mapping (e.g. stack). The
 reason for the failure is that some architectures enforce an alignment
 for the given address hint without MAP_FIXED used (e.g. for shared or
 file backed mappings).

 One way around this would be excluding those archs which do alignment
 tricks from the hardening [4]. The patch is really trivial but it has
 been objected, rightfully so, that this screams for a more generic
 solution. We basically want a non-destructive MAP_FIXED.

 The first patch introduced MAP_FIXED_SAFE which enforces the given
 address but unlike MAP_FIXED it fails with ENOMEM if the given range
 conflicts with an existing one. The flag is introduced as a completely
 new flag rather than a MAP_FIXED extension because of the backward
 compatibility. We really want a never-clobber semantic even on older
 kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
 flags evaluation because we do not EINVAL on unknown flags. On those
 kernels we would simply use the traditional hint based semantic so the
 caller can still get a different address (which sucks) but at least not
 silently corrupt an existing mapping. I do not see a good way around
 that. Except we won't export expose the new semantic to the userspace at
 all. It seems there are users who would like to have something like that
 [5], though. Atomic address range probing in the multithreaded programs
 sounds like an interesting thing to me as well, although I do not have
 any specific usecase in mind.
>>
>> Hi Michal,
>>
>> From looking at the patchset, it seems to me that the new MAP_FIXED_SAFE
>> (or whatever it ends up being named) *would* be passed through from
>> user space. When you say that "we won't export expose the new semantic 
>> to the userspace at all", do you mean that glibc won't add it? Or
>> is there something I'm missing, that prevents that flag from getting
>> from the syscall, to do_mmap()?
> 
> I meant that I could make it an internal flag outside of the map_type
> space. So the userspace will not be able to use it.
>  
>> On the usage: there are cases in user space that could probably make
>> good use of a no-clobber hint to MAP_FIXED. The user space code
>> that surrounds HMM (speaking loosely there--it's really any user space
>> code that manages a unified memory address space, across devices)
>> often ends up using MAP_FIXED, but MAP_FIXED crams several features
>> into one flag: an exact address, an "atomic" switch to the new mapping,
>> and unmapping the old mappings. That's pretty overloaded, so being
>> able to split it up a bit, by removing one of those features, seems
>> useful.
> 
> Yes, atomic address range probing sounds useful. I cannot comment on HMM
> usage but if you have any more specific I would welcome any links to add
> them to the changelog.
> 

Hi Michal,

Yes, it really is useful for user space. I'll use CUDA as an example, but I 
think anything that enforces a uniform virtual addressing scheme across CPUs
and devices, probably has to do something eerily similar. CUDA does this:

a) Searches /proc//maps for a "suitable" region of available VA space. 
"Suitable" generally means it has to have a base address within a certain
limited range (a particular device model might have odd limitations, for 
example), it has to be large enough, and alignment has to be large enough
(again, various devices may have constraints that lead us to do this).

This is of course subject to races with other threads in the process.

Let's say it finds a region starting at va.

b) Next it does: 
p = mmap(va, ...) 

*without* setting MAP_FIXED, of course (so va is just a hint), to attempt to
safely reserve that region. If p != va, then in most cases, this is a failure
(almost certainly due to another thread getting a mapping from that region
before we did), and so this layer now has to call munmap(), before returning
a "failure: retry" to upper layers.

IMPROVEMENT: --> if instead, we could call this:

p = mmap(va, ... MAP_FIXED_NO_CLOBBER ...)

, then we could skip the munmap() call upon failure. This is a small 
thing, 
but it is useful here. (Thanks to Piotr Jaroszynski and Mark Hairgrove
for helping me get that detail exactly right, btw.)

c) After that, CUDA suballocates from p, via: 
 
 q = mmap(sub_region_start, ... MAP_FIXED ...)

Interestingly enough, 

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

On Fri 17-11-17 00:45:49, John Hubbard wrote:
> On 11/16/2017 04:14 AM, Michal Hocko wrote:
> > [Ups, managed to screw the subject - fix it]
> > 
> > On Thu 16-11-17 11:18:58, Michal Hocko wrote:
> >> Hi,
> >> this has started as a follow up discussion [1][2] resulting in the
> >> runtime failure caused by hardening patch [3] which removes MAP_FIXED
> >> from the elf loader because MAP_FIXED is inherently dangerous as it
> >> might silently clobber and existing underlying mapping (e.g. stack). The
> >> reason for the failure is that some architectures enforce an alignment
> >> for the given address hint without MAP_FIXED used (e.g. for shared or
> >> file backed mappings).
> >>
> >> One way around this would be excluding those archs which do alignment
> >> tricks from the hardening [4]. The patch is really trivial but it has
> >> been objected, rightfully so, that this screams for a more generic
> >> solution. We basically want a non-destructive MAP_FIXED.
> >>
> >> The first patch introduced MAP_FIXED_SAFE which enforces the given
> >> address but unlike MAP_FIXED it fails with ENOMEM if the given range
> >> conflicts with an existing one. The flag is introduced as a completely
> >> new flag rather than a MAP_FIXED extension because of the backward
> >> compatibility. We really want a never-clobber semantic even on older
> >> kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
> >> flags evaluation because we do not EINVAL on unknown flags. On those
> >> kernels we would simply use the traditional hint based semantic so the
> >> caller can still get a different address (which sucks) but at least not
> >> silently corrupt an existing mapping. I do not see a good way around
> >> that. Except we won't export expose the new semantic to the userspace at
> >> all. It seems there are users who would like to have something like that
> >> [5], though. Atomic address range probing in the multithreaded programs
> >> sounds like an interesting thing to me as well, although I do not have
> >> any specific usecase in mind.
> 
> Hi Michal,
> 
> From looking at the patchset, it seems to me that the new MAP_FIXED_SAFE
> (or whatever it ends up being named) *would* be passed through from
> user space. When you say that "we won't export expose the new semantic 
> to the userspace at all", do you mean that glibc won't add it? Or
> is there something I'm missing, that prevents that flag from getting
> from the syscall, to do_mmap()?

I meant that I could make it an internal flag outside of the map_type
space. So the userspace will not be able to use it.
 
> On the usage: there are cases in user space that could probably make
> good use of a no-clobber hint to MAP_FIXED. The user space code
> that surrounds HMM (speaking loosely there--it's really any user space
> code that manages a unified memory address space, across devices)
> often ends up using MAP_FIXED, but MAP_FIXED crams several features
> into one flag: an exact address, an "atomic" switch to the new mapping,
> and unmapping the old mappings. That's pretty overloaded, so being
> able to split it up a bit, by removing one of those features, seems
> useful.

Yes, atomic address range probing sounds useful. I cannot comment on HMM
usage but if you have any more specific I would welcome any links to add
them to the changelog.
-- 
Michal Hocko
SUSE Labs

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

On Fri 17-11-17 00:45:49, John Hubbard wrote:
> On 11/16/2017 04:14 AM, Michal Hocko wrote:
> > [Ups, managed to screw the subject - fix it]
> > 
> > On Thu 16-11-17 11:18:58, Michal Hocko wrote:
> >> Hi,
> >> this has started as a follow up discussion [1][2] resulting in the
> >> runtime failure caused by hardening patch [3] which removes MAP_FIXED
> >> from the elf loader because MAP_FIXED is inherently dangerous as it
> >> might silently clobber and existing underlying mapping (e.g. stack). The
> >> reason for the failure is that some architectures enforce an alignment
> >> for the given address hint without MAP_FIXED used (e.g. for shared or
> >> file backed mappings).
> >>
> >> One way around this would be excluding those archs which do alignment
> >> tricks from the hardening [4]. The patch is really trivial but it has
> >> been objected, rightfully so, that this screams for a more generic
> >> solution. We basically want a non-destructive MAP_FIXED.
> >>
> >> The first patch introduced MAP_FIXED_SAFE which enforces the given
> >> address but unlike MAP_FIXED it fails with ENOMEM if the given range
> >> conflicts with an existing one. The flag is introduced as a completely
> >> new flag rather than a MAP_FIXED extension because of the backward
> >> compatibility. We really want a never-clobber semantic even on older
> >> kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
> >> flags evaluation because we do not EINVAL on unknown flags. On those
> >> kernels we would simply use the traditional hint based semantic so the
> >> caller can still get a different address (which sucks) but at least not
> >> silently corrupt an existing mapping. I do not see a good way around
> >> that. Except we won't export expose the new semantic to the userspace at
> >> all. It seems there are users who would like to have something like that
> >> [5], though. Atomic address range probing in the multithreaded programs
> >> sounds like an interesting thing to me as well, although I do not have
> >> any specific usecase in mind.
> 
> Hi Michal,
> 
> From looking at the patchset, it seems to me that the new MAP_FIXED_SAFE
> (or whatever it ends up being named) *would* be passed through from
> user space. When you say that "we won't export expose the new semantic 
> to the userspace at all", do you mean that glibc won't add it? Or
> is there something I'm missing, that prevents that flag from getting
> from the syscall, to do_mmap()?

I meant that I could make it an internal flag outside of the map_type
space. So the userspace will not be able to use it.
 
> On the usage: there are cases in user space that could probably make
> good use of a no-clobber hint to MAP_FIXED. The user space code
> that surrounds HMM (speaking loosely there--it's really any user space
> code that manages a unified memory address space, across devices)
> often ends up using MAP_FIXED, but MAP_FIXED crams several features
> into one flag: an exact address, an "atomic" switch to the new mapping,
> and unmapping the old mappings. That's pretty overloaded, so being
> able to split it up a bit, by removing one of those features, seems
> useful.

Yes, atomic address range probing sounds useful. I cannot comment on HMM
usage but if you have any more specific I would welcome any links to add
them to the changelog.
-- 
Michal Hocko
SUSE Labs

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[John Hubbard]

On 11/16/2017 04:14 AM, Michal Hocko wrote:
> [Ups, managed to screw the subject - fix it]
> 
> On Thu 16-11-17 11:18:58, Michal Hocko wrote:
>> Hi,
>> this has started as a follow up discussion [1][2] resulting in the
>> runtime failure caused by hardening patch [3] which removes MAP_FIXED
>> from the elf loader because MAP_FIXED is inherently dangerous as it
>> might silently clobber and existing underlying mapping (e.g. stack). The
>> reason for the failure is that some architectures enforce an alignment
>> for the given address hint without MAP_FIXED used (e.g. for shared or
>> file backed mappings).
>>
>> One way around this would be excluding those archs which do alignment
>> tricks from the hardening [4]. The patch is really trivial but it has
>> been objected, rightfully so, that this screams for a more generic
>> solution. We basically want a non-destructive MAP_FIXED.
>>
>> The first patch introduced MAP_FIXED_SAFE which enforces the given
>> address but unlike MAP_FIXED it fails with ENOMEM if the given range
>> conflicts with an existing one. The flag is introduced as a completely
>> new flag rather than a MAP_FIXED extension because of the backward
>> compatibility. We really want a never-clobber semantic even on older
>> kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
>> flags evaluation because we do not EINVAL on unknown flags. On those
>> kernels we would simply use the traditional hint based semantic so the
>> caller can still get a different address (which sucks) but at least not
>> silently corrupt an existing mapping. I do not see a good way around
>> that. Except we won't export expose the new semantic to the userspace at
>> all. It seems there are users who would like to have something like that
>> [5], though. Atomic address range probing in the multithreaded programs
>> sounds like an interesting thing to me as well, although I do not have
>> any specific usecase in mind.

Hi Michal,

>From looking at the patchset, it seems to me that the new MAP_FIXED_SAFE
(or whatever it ends up being named) *would* be passed through from
user space. When you say that "we won't export expose the new semantic 
to the userspace at all", do you mean that glibc won't add it? Or
is there something I'm missing, that prevents that flag from getting
from the syscall, to do_mmap()?

On the usage: there are cases in user space that could probably make
good use of a no-clobber hint to MAP_FIXED. The user space code
that surrounds HMM (speaking loosely there--it's really any user space
code that manages a unified memory address space, across devices)
often ends up using MAP_FIXED, but MAP_FIXED crams several features
into one flag: an exact address, an "atomic" switch to the new mapping,
and unmapping the old mappings. That's pretty overloaded, so being
able to split it up a bit, by removing one of those features, seems
useful.

thanks,
John Hubbard

>>
>> The second patch simply replaces MAP_FIXED use in elf loader by
>> MAP_FIXED_SAFE. I believe other places which rely on MAP_FIXED should
>> follow. Actually real MAP_FIXED usages should be docummented properly
>> and they should be more of an exception.
>>
>> Does anybody see any fundamental reasons why this is a wrong approach?
>>
>> Diffstat says
>>  arch/alpha/include/uapi/asm/mman.h   |  2 ++
>>  arch/metag/kernel/process.c  |  6 +-
>>  arch/mips/include/uapi/asm/mman.h|  2 ++
>>  arch/parisc/include/uapi/asm/mman.h  |  2 ++
>>  arch/powerpc/include/uapi/asm/mman.h |  1 +
>>  arch/sparc/include/uapi/asm/mman.h   |  1 +
>>  arch/tile/include/uapi/asm/mman.h|  1 +
>>  arch/xtensa/include/uapi/asm/mman.h  |  2 ++
>>  fs/binfmt_elf.c  | 12 
>>  include/uapi/asm-generic/mman.h  |  1 +
>>  mm/mmap.c| 11 +++
>>  11 files changed, 36 insertions(+), 5 deletions(-)
>>
>> [1] http://lkml.kernel.org/r/20171107162217.382cd...@canb.auug.org.au
>> [2] http://lkml.kernel.org/r/1510048229.12079.7.ca...@abdul.in.ibm.com
>> [3] http://lkml.kernel.org/r/20171023082608.6167-1-mho...@kernel.org
>> [4] http://lkml.kernel.org/r/20171113094203.aofz2e7kueitk...@dhcp22.suse.cz
>> [5] http://lkml.kernel.org/r/87efp1w7vy@concordia.ellerman.id.au
>>
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majord...@kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 
> 

Re: [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[John Hubbard]

On 11/16/2017 04:14 AM, Michal Hocko wrote:
> [Ups, managed to screw the subject - fix it]
> 
> On Thu 16-11-17 11:18:58, Michal Hocko wrote:
>> Hi,
>> this has started as a follow up discussion [1][2] resulting in the
>> runtime failure caused by hardening patch [3] which removes MAP_FIXED
>> from the elf loader because MAP_FIXED is inherently dangerous as it
>> might silently clobber and existing underlying mapping (e.g. stack). The
>> reason for the failure is that some architectures enforce an alignment
>> for the given address hint without MAP_FIXED used (e.g. for shared or
>> file backed mappings).
>>
>> One way around this would be excluding those archs which do alignment
>> tricks from the hardening [4]. The patch is really trivial but it has
>> been objected, rightfully so, that this screams for a more generic
>> solution. We basically want a non-destructive MAP_FIXED.
>>
>> The first patch introduced MAP_FIXED_SAFE which enforces the given
>> address but unlike MAP_FIXED it fails with ENOMEM if the given range
>> conflicts with an existing one. The flag is introduced as a completely
>> new flag rather than a MAP_FIXED extension because of the backward
>> compatibility. We really want a never-clobber semantic even on older
>> kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
>> flags evaluation because we do not EINVAL on unknown flags. On those
>> kernels we would simply use the traditional hint based semantic so the
>> caller can still get a different address (which sucks) but at least not
>> silently corrupt an existing mapping. I do not see a good way around
>> that. Except we won't export expose the new semantic to the userspace at
>> all. It seems there are users who would like to have something like that
>> [5], though. Atomic address range probing in the multithreaded programs
>> sounds like an interesting thing to me as well, although I do not have
>> any specific usecase in mind.

Hi Michal,

>From looking at the patchset, it seems to me that the new MAP_FIXED_SAFE
(or whatever it ends up being named) *would* be passed through from
user space. When you say that "we won't export expose the new semantic 
to the userspace at all", do you mean that glibc won't add it? Or
is there something I'm missing, that prevents that flag from getting
from the syscall, to do_mmap()?

On the usage: there are cases in user space that could probably make
good use of a no-clobber hint to MAP_FIXED. The user space code
that surrounds HMM (speaking loosely there--it's really any user space
code that manages a unified memory address space, across devices)
often ends up using MAP_FIXED, but MAP_FIXED crams several features
into one flag: an exact address, an "atomic" switch to the new mapping,
and unmapping the old mappings. That's pretty overloaded, so being
able to split it up a bit, by removing one of those features, seems
useful.

thanks,
John Hubbard

>>
>> The second patch simply replaces MAP_FIXED use in elf loader by
>> MAP_FIXED_SAFE. I believe other places which rely on MAP_FIXED should
>> follow. Actually real MAP_FIXED usages should be docummented properly
>> and they should be more of an exception.
>>
>> Does anybody see any fundamental reasons why this is a wrong approach?
>>
>> Diffstat says
>>  arch/alpha/include/uapi/asm/mman.h   |  2 ++
>>  arch/metag/kernel/process.c  |  6 +-
>>  arch/mips/include/uapi/asm/mman.h|  2 ++
>>  arch/parisc/include/uapi/asm/mman.h  |  2 ++
>>  arch/powerpc/include/uapi/asm/mman.h |  1 +
>>  arch/sparc/include/uapi/asm/mman.h   |  1 +
>>  arch/tile/include/uapi/asm/mman.h|  1 +
>>  arch/xtensa/include/uapi/asm/mman.h  |  2 ++
>>  fs/binfmt_elf.c  | 12 
>>  include/uapi/asm-generic/mman.h  |  1 +
>>  mm/mmap.c| 11 +++
>>  11 files changed, 36 insertions(+), 5 deletions(-)
>>
>> [1] http://lkml.kernel.org/r/20171107162217.382cd...@canb.auug.org.au
>> [2] http://lkml.kernel.org/r/1510048229.12079.7.ca...@abdul.in.ibm.com
>> [3] http://lkml.kernel.org/r/20171023082608.6167-1-mho...@kernel.org
>> [4] http://lkml.kernel.org/r/20171113094203.aofz2e7kueitk...@dhcp22.suse.cz
>> [5] http://lkml.kernel.org/r/87efp1w7vy@concordia.ellerman.id.au
>>
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majord...@kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 
> 

[RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

[Ups, managed to screw the subject - fix it]

On Thu 16-11-17 11:18:58, Michal Hocko wrote:
> Hi,
> this has started as a follow up discussion [1][2] resulting in the
> runtime failure caused by hardening patch [3] which removes MAP_FIXED
> from the elf loader because MAP_FIXED is inherently dangerous as it
> might silently clobber and existing underlying mapping (e.g. stack). The
> reason for the failure is that some architectures enforce an alignment
> for the given address hint without MAP_FIXED used (e.g. for shared or
> file backed mappings).
> 
> One way around this would be excluding those archs which do alignment
> tricks from the hardening [4]. The patch is really trivial but it has
> been objected, rightfully so, that this screams for a more generic
> solution. We basically want a non-destructive MAP_FIXED.
> 
> The first patch introduced MAP_FIXED_SAFE which enforces the given
> address but unlike MAP_FIXED it fails with ENOMEM if the given range
> conflicts with an existing one. The flag is introduced as a completely
> new flag rather than a MAP_FIXED extension because of the backward
> compatibility. We really want a never-clobber semantic even on older
> kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
> flags evaluation because we do not EINVAL on unknown flags. On those
> kernels we would simply use the traditional hint based semantic so the
> caller can still get a different address (which sucks) but at least not
> silently corrupt an existing mapping. I do not see a good way around
> that. Except we won't export expose the new semantic to the userspace at
> all. It seems there are users who would like to have something like that
> [5], though. Atomic address range probing in the multithreaded programs
> sounds like an interesting thing to me as well, although I do not have
> any specific usecase in mind.
> 
> The second patch simply replaces MAP_FIXED use in elf loader by
> MAP_FIXED_SAFE. I believe other places which rely on MAP_FIXED should
> follow. Actually real MAP_FIXED usages should be docummented properly
> and they should be more of an exception.
> 
> Does anybody see any fundamental reasons why this is a wrong approach?
> 
> Diffstat says
>  arch/alpha/include/uapi/asm/mman.h   |  2 ++
>  arch/metag/kernel/process.c  |  6 +-
>  arch/mips/include/uapi/asm/mman.h|  2 ++
>  arch/parisc/include/uapi/asm/mman.h  |  2 ++
>  arch/powerpc/include/uapi/asm/mman.h |  1 +
>  arch/sparc/include/uapi/asm/mman.h   |  1 +
>  arch/tile/include/uapi/asm/mman.h|  1 +
>  arch/xtensa/include/uapi/asm/mman.h  |  2 ++
>  fs/binfmt_elf.c  | 12 
>  include/uapi/asm-generic/mman.h  |  1 +
>  mm/mmap.c| 11 +++
>  11 files changed, 36 insertions(+), 5 deletions(-)
> 
> [1] http://lkml.kernel.org/r/20171107162217.382cd...@canb.auug.org.au
> [2] http://lkml.kernel.org/r/1510048229.12079.7.ca...@abdul.in.ibm.com
> [3] http://lkml.kernel.org/r/20171023082608.6167-1-mho...@kernel.org
> [4] http://lkml.kernel.org/r/20171113094203.aofz2e7kueitk...@dhcp22.suse.cz
> [5] http://lkml.kernel.org/r/87efp1w7vy@concordia.ellerman.id.au
> 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

-- 
Michal Hocko
SUSE Labs

[RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Michal Hocko]

[Ups, managed to screw the subject - fix it]

On Thu 16-11-17 11:18:58, Michal Hocko wrote:
> Hi,
> this has started as a follow up discussion [1][2] resulting in the
> runtime failure caused by hardening patch [3] which removes MAP_FIXED
> from the elf loader because MAP_FIXED is inherently dangerous as it
> might silently clobber and existing underlying mapping (e.g. stack). The
> reason for the failure is that some architectures enforce an alignment
> for the given address hint without MAP_FIXED used (e.g. for shared or
> file backed mappings).
> 
> One way around this would be excluding those archs which do alignment
> tricks from the hardening [4]. The patch is really trivial but it has
> been objected, rightfully so, that this screams for a more generic
> solution. We basically want a non-destructive MAP_FIXED.
> 
> The first patch introduced MAP_FIXED_SAFE which enforces the given
> address but unlike MAP_FIXED it fails with ENOMEM if the given range
> conflicts with an existing one. The flag is introduced as a completely
> new flag rather than a MAP_FIXED extension because of the backward
> compatibility. We really want a never-clobber semantic even on older
> kernels which do not recognize the flag. Unfortunately mmap sucks wrt.
> flags evaluation because we do not EINVAL on unknown flags. On those
> kernels we would simply use the traditional hint based semantic so the
> caller can still get a different address (which sucks) but at least not
> silently corrupt an existing mapping. I do not see a good way around
> that. Except we won't export expose the new semantic to the userspace at
> all. It seems there are users who would like to have something like that
> [5], though. Atomic address range probing in the multithreaded programs
> sounds like an interesting thing to me as well, although I do not have
> any specific usecase in mind.
> 
> The second patch simply replaces MAP_FIXED use in elf loader by
> MAP_FIXED_SAFE. I believe other places which rely on MAP_FIXED should
> follow. Actually real MAP_FIXED usages should be docummented properly
> and they should be more of an exception.
> 
> Does anybody see any fundamental reasons why this is a wrong approach?
> 
> Diffstat says
>  arch/alpha/include/uapi/asm/mman.h   |  2 ++
>  arch/metag/kernel/process.c  |  6 +-
>  arch/mips/include/uapi/asm/mman.h|  2 ++
>  arch/parisc/include/uapi/asm/mman.h  |  2 ++
>  arch/powerpc/include/uapi/asm/mman.h |  1 +
>  arch/sparc/include/uapi/asm/mman.h   |  1 +
>  arch/tile/include/uapi/asm/mman.h|  1 +
>  arch/xtensa/include/uapi/asm/mman.h  |  2 ++
>  fs/binfmt_elf.c  | 12 
>  include/uapi/asm-generic/mman.h  |  1 +
>  mm/mmap.c| 11 +++
>  11 files changed, 36 insertions(+), 5 deletions(-)
> 
> [1] http://lkml.kernel.org/r/20171107162217.382cd...@canb.auug.org.au
> [2] http://lkml.kernel.org/r/1510048229.12079.7.ca...@abdul.in.ibm.com
> [3] http://lkml.kernel.org/r/20171023082608.6167-1-mho...@kernel.org
> [4] http://lkml.kernel.org/r/20171113094203.aofz2e7kueitk...@dhcp22.suse.cz
> [5] http://lkml.kernel.org/r/87efp1w7vy@concordia.ellerman.id.au
> 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

-- 
Michal Hocko
SUSE Labs