Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE
On Fri, Dec 08, 2017 at 12:13:31PM -0800, Kees Cook wrote: > On Fri, Dec 8, 2017 at 12:33 AM, Michal Hockowrote: > > 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
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
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
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
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
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
On Fri, Dec 8, 2017 at 12:33 AM, Michal Hockowrote: > 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
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
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
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
On Fri 2017-12-08 22:08:07, Michael Ellerman wrote: > Matthew Wilcoxwrites: > > > 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
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
Matthew Wilcoxwrites: > 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
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
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
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
On Thu, Dec 07, 2017 at 11:14:27AM -0800, Kees Cook wrote: > On Wed, Dec 6, 2017 at 9:46 PM, Michael Ellermanwrote: > > 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
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
On Wed, Dec 6, 2017 at 9:46 PM, Michael Ellermanwrote: > 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
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
Matthew Wilcoxwrites: > 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
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
On 12/06/2017 04:19 PM, Kees Cook wrote: > On Wed, Dec 6, 2017 at 1:08 AM, Michal Hockowrote: >> 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
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
On Wed, Dec 6, 2017 at 1:08 AM, Michal Hockowrote: > 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
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
On Wed 06-12-17 08:33:37, Rasmus Villemoes wrote: > On 2017-12-06 05:50, Michael Ellerman wrote: > > Michal Hockowrites: > > > >> 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
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
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
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
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
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
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
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
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 Hrubiswrites: >>> 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
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
On 2017-12-06 05:50, Michael Ellerman wrote: > Michal Hockowrites: > >> 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
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
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 Hrubiswrites: > > > > > 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
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
On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote: > Cyril Hrubiswrites: > > > 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
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
Cyril Hrubiswrites: > 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
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
Michal Hockowrites: > 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
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
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
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
On Wed 29-11-17 14:25:36, Kees Cook wrote: > On Wed, Nov 29, 2017 at 6:42 AM, Michal Hockowrote: > > 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
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
On Wed, Nov 29, 2017 at 6:42 AM, Michal Hockowrote: > 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
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
On Wed, Nov 29, 2017 at 7:13 AM, Rasmus Villemoeswrote: > 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
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
On Wed, Nov 29, 2017 at 6:42 AM, Michal Hockowrote: > 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
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
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
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
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
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