LinuxLists.cc - [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Thu, Apr 12, 2018 at 8:37 PM, Michael Kerrisk (man-pages)
<[email protected]> wrote:
> Hi John,
>
> On 12 April 2018 at 20:33, John Hubbard <[email protected]> wrote:
>> On 04/12/2018 08:39 AM, Jann Horn wrote:
>>> Clarify that MAP_FIXED is appropriate if the specified address range has
>>> been reserved using an existing mapping, but shouldn't be used otherwise.
>>>
>>> Signed-off-by: Jann Horn <[email protected]>
>>> ---
>>> man2/mmap.2 | 19 +++++++++++--------
>>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/man2/mmap.2 b/man2/mmap.2
[...]
>>> .IP
>>> For example, suppose that thread A looks through
>>> @@ -284,13 +285,15 @@ and the PAM libraries
>>> .UR http://www.linux-pam.org
>>> .UE .
>>> .IP
>>> -Newer kernels
>>> -(Linux 4.17 and later) have a
>>> +For cases in which the specified memory region has not been reserved using an
>>> +existing mapping, newer kernels (Linux 4.17 and later) provide an option
>>> .B MAP_FIXED_NOREPLACE
>>> -option that avoids the corruption problem; if available,
>>> -.B MAP_FIXED_NOREPLACE
>>> -should be preferred over
>>> -.BR MAP_FIXED .
>>> +that should be used instead; older kernels require the caller to use
>>> +.I addr
>>> +as a hint (without
>>> +.BR MAP_FIXED )
>>
>> Here, I got lost: the sentence suddenly jumps into explaining non-MAP_FIXED
>> behavior, in the MAP_FIXED section. Maybe if you break up the sentence, and
>> possibly omit non-MAP_FIXED discussion, it will help.
>
> Hmmm -- true. That piece could be a little clearer.

How about something like this?

For cases in which MAP_FIXED can not be used because
the specified memory
region has not been reserved using an existing mapping,
newer kernels
(Linux 4.17 and later) provide an option
MAP_FIXED_NOREPLACE that
should be used instead. Older kernels require the
caller to use addr as a hint and take appropriate action if
the kernel places the new mapping at a different address.

John, Michael, what do you think?

> Jann, I've already pushed the existing patch. Do you want to add a patch on top?

2018-04-12 19:00:49

by John Hubbard

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On 04/12/2018 11:49 AM, Jann Horn wrote:
> On Thu, Apr 12, 2018 at 8:37 PM, Michael Kerrisk (man-pages)
> <[email protected]> wrote:
>> Hi John,
>>
>> On 12 April 2018 at 20:33, John Hubbard <[email protected]> wrote:
>>> On 04/12/2018 08:39 AM, Jann Horn wrote:
>>>> Clarify that MAP_FIXED is appropriate if the specified address range has
>>>> been reserved using an existing mapping, but shouldn't be used otherwise.
>>>>
>>>> Signed-off-by: Jann Horn <[email protected]>
>>>> ---
>>>> man2/mmap.2 | 19 +++++++++++--------
>>>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/man2/mmap.2 b/man2/mmap.2
> [...]
>>>> .IP
>>>> For example, suppose that thread A looks through
>>>> @@ -284,13 +285,15 @@ and the PAM libraries
>>>> .UR http://www.linux-pam.org
>>>> .UE .
>>>> .IP
>>>> -Newer kernels
>>>> -(Linux 4.17 and later) have a
>>>> +For cases in which the specified memory region has not been reserved using an
>>>> +existing mapping, newer kernels (Linux 4.17 and later) provide an option
>>>> .B MAP_FIXED_NOREPLACE
>>>> -option that avoids the corruption problem; if available,
>>>> -.B MAP_FIXED_NOREPLACE
>>>> -should be preferred over
>>>> -.BR MAP_FIXED .
>>>> +that should be used instead; older kernels require the caller to use
>>>> +.I addr
>>>> +as a hint (without
>>>> +.BR MAP_FIXED )
>>>
>>> Here, I got lost: the sentence suddenly jumps into explaining non-MAP_FIXED
>>> behavior, in the MAP_FIXED section. Maybe if you break up the sentence, and
>>> possibly omit non-MAP_FIXED discussion, it will help.
>>
>> Hmmm -- true. That piece could be a little clearer.
>
> How about something like this?
>
> For cases in which MAP_FIXED can not be used because
> the specified memory
> region has not been reserved using an existing mapping,
> newer kernels
> (Linux 4.17 and later) provide an option
> MAP_FIXED_NOREPLACE that
> should be used instead. Older kernels require the
> caller to use addr as a hint and take appropriate action if
> the kernel places the new mapping at a different address.
>
> John, Michael, what do you think?

I'm still having difficulty with it, because this is in the MAP_FIXED section,
but I think you're documenting the behavior that you get if you do *not*
specify MAP_FIXED, right? Also, the hint behavior is true of both older and
new kernels...

So, if that's your intent (you want to sort of document by contrast to what
would happen if this option were not used), then how about something like this:

Without the MAP_FIXED option, the kernel would treat addr as a hint, rather
than a requirement, and the caller would need to take appropriate action
if the kernel placed the mapping at a different address. (For example,
munmap and try again.)

thanks,
--
John Hubbard
NVIDIA

2018-04-12 19:26:08

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Thu, Apr 12, 2018 at 8:59 PM, John Hubbard <[email protected]> wrote:
> On 04/12/2018 11:49 AM, Jann Horn wrote:
>> On Thu, Apr 12, 2018 at 8:37 PM, Michael Kerrisk (man-pages)
>> <[email protected]> wrote:
>>> Hi John,
>>>
>>> On 12 April 2018 at 20:33, John Hubbard <[email protected]> wrote:
>>>> On 04/12/2018 08:39 AM, Jann Horn wrote:
>>>>> Clarify that MAP_FIXED is appropriate if the specified address range has
>>>>> been reserved using an existing mapping, but shouldn't be used otherwise.
>>>>>
>>>>> Signed-off-by: Jann Horn <[email protected]>
>>>>> ---
>>>>> man2/mmap.2 | 19 +++++++++++--------
>>>>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/man2/mmap.2 b/man2/mmap.2
>> [...]
>>>>> .IP
>>>>> For example, suppose that thread A looks through
>>>>> @@ -284,13 +285,15 @@ and the PAM libraries
>>>>> .UR http://www.linux-pam.org
>>>>> .UE .
>>>>> .IP
>>>>> -Newer kernels
>>>>> -(Linux 4.17 and later) have a
>>>>> +For cases in which the specified memory region has not been reserved using an
>>>>> +existing mapping, newer kernels (Linux 4.17 and later) provide an option
>>>>> .B MAP_FIXED_NOREPLACE
>>>>> -option that avoids the corruption problem; if available,
>>>>> -.B MAP_FIXED_NOREPLACE
>>>>> -should be preferred over
>>>>> -.BR MAP_FIXED .
>>>>> +that should be used instead; older kernels require the caller to use
>>>>> +.I addr
>>>>> +as a hint (without
>>>>> +.BR MAP_FIXED )
>>>>
>>>> Here, I got lost: the sentence suddenly jumps into explaining non-MAP_FIXED
>>>> behavior, in the MAP_FIXED section. Maybe if you break up the sentence, and
>>>> possibly omit non-MAP_FIXED discussion, it will help.
>>>
>>> Hmmm -- true. That piece could be a little clearer.
>>
>> How about something like this?
>>
>> For cases in which MAP_FIXED can not be used because
>> the specified memory
>> region has not been reserved using an existing mapping,
>> newer kernels
>> (Linux 4.17 and later) provide an option
>> MAP_FIXED_NOREPLACE that
>> should be used instead. Older kernels require the
>> caller to use addr as a hint and take appropriate action if
>> the kernel places the new mapping at a different address.
>>
>> John, Michael, what do you think?
>
>
> I'm still having difficulty with it, because this is in the MAP_FIXED section,
> but I think you're documenting the behavior that you get if you do *not*
> specify MAP_FIXED, right? Also, the hint behavior is true of both older and
> new kernels...

The manpage patch you and mhocko wrote mentioned MAP_FIXED_NOREPLACE
in the MAP_FIXED section - I was trying to avoid undoing a change you
had just explicitly made.

> So, if that's your intent (you want to sort of document by contrast to what
> would happen if this option were not used), then how about something like this:
>
>
> Without the MAP_FIXED option, the kernel would treat addr as a hint, rather
> than a requirement, and the caller would need to take appropriate action
> if the kernel placed the mapping at a different address. (For example,
> munmap and try again.)

I'd be fine with removing the paragraph. As you rightly pointed out,
it doesn't really describe MAP_FIXED.

2018-04-12 19:44:09

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

Hello Jann,

On 04/12/2018 05:39 PM, Jann Horn wrote:
> Clarify that MAP_FIXED is appropriate if the specified address range has
> been reserved using an existing mapping, but shouldn't be used otherwise.
>
> Signed-off-by: Jann Horn <[email protected]>
> ---
> man2/mmap.2 | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index bef8b4432..80c9ec285 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -253,8 +253,9 @@ Software that aspires to be portable should use this option with care,
> keeping in mind that the exact layout of a process's memory mappings
> is allowed to change significantly between kernel versions,
> C library versions, and operating system releases.
> -Furthermore, this option is extremely hazardous (when used on its own),
> -because it forcibly removes preexisting mappings,
> +This option should only be used when the specified memory region has
> +already been reserved using another mapping; otherwise, it is extremely
> +hazardous because it forcibly removes preexisting mappings,
> making it easy for a multithreaded process to corrupt its own address space.
> .IP
> For example, suppose that thread A looks through
> @@ -284,13 +285,15 @@ and the PAM libraries
> .UR http://www.linux-pam.org
> .UE .
> .IP
> -Newer kernels
> -(Linux 4.17 and later) have a
> +For cases in which the specified memory region has not been reserved using an
> +existing mapping, newer kernels (Linux 4.17 and later) provide an option
> .B MAP_FIXED_NOREPLACE
> -option that avoids the corruption problem; if available,
> -.B MAP_FIXED_NOREPLACE
> -should be preferred over
> -.BR MAP_FIXED .
> +that should be used instead; older kernels require the caller to use
> +.I addr
> +as a hint (without
> +.BR MAP_FIXED )
> +and take appropriate action if the kernel places the new mapping at a
> +different address.
> .TP
> .BR MAP_FIXED_NOREPLACE " (since Linux 4.17)"
> .\" commit a4ff8e8620d3f4f50ac4b41e8067b7d395056843

Thanks! Nice patch! Applied.

Cheers,

Michael

--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

2018-04-12 19:44:46

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

Hi John,

On 12 April 2018 at 20:33, John Hubbard <[email protected]> wrote:
> On 04/12/2018 08:39 AM, Jann Horn wrote:
>> Clarify that MAP_FIXED is appropriate if the specified address range has
>> been reserved using an existing mapping, but shouldn't be used otherwise.
>>
>> Signed-off-by: Jann Horn <[email protected]>
>> ---
>> man2/mmap.2 | 19 +++++++++++--------
>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/man2/mmap.2 b/man2/mmap.2
>> index bef8b4432..80c9ec285 100644
>> --- a/man2/mmap.2
>> +++ b/man2/mmap.2
>> @@ -253,8 +253,9 @@ Software that aspires to be portable should use this option with care,
>> keeping in mind that the exact layout of a process's memory mappings
>> is allowed to change significantly between kernel versions,
>> C library versions, and operating system releases.
>> -Furthermore, this option is extremely hazardous (when used on its own),
>> -because it forcibly removes preexisting mappings,
>> +This option should only be used when the specified memory region has
>> +already been reserved using another mapping; otherwise, it is extremely
>> +hazardous because it forcibly removes preexisting mappings,
>> making it easy for a multithreaded process to corrupt its own address space.
>
> Yes, that's clearer and provides more information than before.
>
>> .IP
>> For example, suppose that thread A looks through
>> @@ -284,13 +285,15 @@ and the PAM libraries
>> .UR http://www.linux-pam.org
>> .UE .
>> .IP
>> -Newer kernels
>> -(Linux 4.17 and later) have a
>> +For cases in which the specified memory region has not been reserved using an
>> +existing mapping, newer kernels (Linux 4.17 and later) provide an option
>> .B MAP_FIXED_NOREPLACE
>> -option that avoids the corruption problem; if available,
>> -.B MAP_FIXED_NOREPLACE
>> -should be preferred over
>> -.BR MAP_FIXED .
>> +that should be used instead; older kernels require the caller to use
>> +.I addr
>> +as a hint (without
>> +.BR MAP_FIXED )
>
> Here, I got lost: the sentence suddenly jumps into explaining non-MAP_FIXED
> behavior, in the MAP_FIXED section. Maybe if you break up the sentence, and
> possibly omit non-MAP_FIXED discussion, it will help.

Hmmm -- true. That piece could be a little clearer.

Jann, I've already pushed the existing patch. Do you want to add a patch on top?

Thanks,

Michael

--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

2018-04-12 19:52:11

by John Hubbard

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On 04/12/2018 12:18 PM, Jann Horn wrote:
> On Thu, Apr 12, 2018 at 8:59 PM, John Hubbard <[email protected]> wrote:
>> On 04/12/2018 11:49 AM, Jann Horn wrote:
>>> On Thu, Apr 12, 2018 at 8:37 PM, Michael Kerrisk (man-pages)
>>> <[email protected]> wrote:
>>>> Hi John,
>>>>
>>>> On 12 April 2018 at 20:33, John Hubbard <[email protected]> wrote:
>>>>> On 04/12/2018 08:39 AM, Jann Horn wrote:
>>>>>> Clarify that MAP_FIXED is appropriate if the specified address range has
>>>>>> been reserved using an existing mapping, but shouldn't be used otherwise.
>>>>>>
>>>>>> Signed-off-by: Jann Horn <[email protected]>
>>>>>> ---
>>>>>> man2/mmap.2 | 19 +++++++++++--------
>>>>>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> diff --git a/man2/mmap.2 b/man2/mmap.2
>>> [...]
>>>>>> .IP
>>>>>> For example, suppose that thread A looks through
>>>>>> @@ -284,13 +285,15 @@ and the PAM libraries
>>>>>> .UR http://www.linux-pam.org
>>>>>> .UE .
>>>>>> .IP
>>>>>> -Newer kernels
>>>>>> -(Linux 4.17 and later) have a
>>>>>> +For cases in which the specified memory region has not been reserved using an
>>>>>> +existing mapping, newer kernels (Linux 4.17 and later) provide an option
>>>>>> .B MAP_FIXED_NOREPLACE
>>>>>> -option that avoids the corruption problem; if available,
>>>>>> -.B MAP_FIXED_NOREPLACE
>>>>>> -should be preferred over
>>>>>> -.BR MAP_FIXED .
>>>>>> +that should be used instead; older kernels require the caller to use
>>>>>> +.I addr
>>>>>> +as a hint (without
>>>>>> +.BR MAP_FIXED )
>>>>>
>>>>> Here, I got lost: the sentence suddenly jumps into explaining non-MAP_FIXED
>>>>> behavior, in the MAP_FIXED section. Maybe if you break up the sentence, and
>>>>> possibly omit non-MAP_FIXED discussion, it will help.
>>>>
>>>> Hmmm -- true. That piece could be a little clearer.
>>>
>>> How about something like this?
>>>
>>> For cases in which MAP_FIXED can not be used because
>>> the specified memory
>>> region has not been reserved using an existing mapping,
>>> newer kernels
>>> (Linux 4.17 and later) provide an option
>>> MAP_FIXED_NOREPLACE that
>>> should be used instead. Older kernels require the
>>> caller to use addr as a hint and take appropriate action if
>>> the kernel places the new mapping at a different address.
>>>
>>> John, Michael, what do you think?
>>
>>
>> I'm still having difficulty with it, because this is in the MAP_FIXED section,
>> but I think you're documenting the behavior that you get if you do *not*
>> specify MAP_FIXED, right? Also, the hint behavior is true of both older and
>> new kernels...
>
> The manpage patch you and mhocko wrote mentioned MAP_FIXED_NOREPLACE
> in the MAP_FIXED section - I was trying to avoid undoing a change you
> had just explicitly made.

heh. So I've succeeding in getting my own wording removed, then? Progress! :)

>
>> So, if that's your intent (you want to sort of document by contrast to what
>> would happen if this option were not used), then how about something like this:
>>
>>
>> Without the MAP_FIXED option, the kernel would treat addr as a hint, rather
>> than a requirement, and the caller would need to take appropriate action
>> if the kernel placed the mapping at a different address. (For example,
>> munmap and try again.)
>
> I'd be fine with removing the paragraph. As you rightly pointed out,
> it doesn't really describe MAP_FIXED.
>

OK, that's probably the simplest fix.

thanks,
--
John Hubbard
NVIDIA

2018-04-13 06:45:33

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On 04/12/2018 09:24 PM, John Hubbard wrote:
> On 04/12/2018 12:18 PM, Jann Horn wrote:
>> On Thu, Apr 12, 2018 at 8:59 PM, John Hubbard <[email protected]> wrote:
>>> On 04/12/2018 11:49 AM, Jann Horn wrote:
>>>> On Thu, Apr 12, 2018 at 8:37 PM, Michael Kerrisk (man-pages)
>>>> <[email protected]> wrote:
>>>>> Hi John,
>>>>>
>>>>> On 12 April 2018 at 20:33, John Hubbard <[email protected]> wrote:
>>>>>> On 04/12/2018 08:39 AM, Jann Horn wrote:
>>>>>>> Clarify that MAP_FIXED is appropriate if the specified address range has
>>>>>>> been reserved using an existing mapping, but shouldn't be used otherwise.
>>>>>>>
>>>>>>> Signed-off-by: Jann Horn <[email protected]>
>>>>>>> ---
>>>>>>> man2/mmap.2 | 19 +++++++++++--------
>>>>>>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>> diff --git a/man2/mmap.2 b/man2/mmap.2
>>>> [...]
>>>>>>> .IP
>>>>>>> For example, suppose that thread A looks through
>>>>>>> @@ -284,13 +285,15 @@ and the PAM libraries
>>>>>>> .UR http://www.linux-pam.org
>>>>>>> .UE .
>>>>>>> .IP
>>>>>>> -Newer kernels
>>>>>>> -(Linux 4.17 and later) have a
>>>>>>> +For cases in which the specified memory region has not been reserved using an
>>>>>>> +existing mapping, newer kernels (Linux 4.17 and later) provide an option
>>>>>>> .B MAP_FIXED_NOREPLACE
>>>>>>> -option that avoids the corruption problem; if available,
>>>>>>> -.B MAP_FIXED_NOREPLACE
>>>>>>> -should be preferred over
>>>>>>> -.BR MAP_FIXED .
>>>>>>> +that should be used instead; older kernels require the caller to use
>>>>>>> +.I addr
>>>>>>> +as a hint (without
>>>>>>> +.BR MAP_FIXED )
>>>>>>
>>>>>> Here, I got lost: the sentence suddenly jumps into explaining non-MAP_FIXED
>>>>>> behavior, in the MAP_FIXED section. Maybe if you break up the sentence, and
>>>>>> possibly omit non-MAP_FIXED discussion, it will help.
>>>>>
>>>>> Hmmm -- true. That piece could be a little clearer.
>>>>
>>>> How about something like this?
>>>>
>>>> For cases in which MAP_FIXED can not be used because
>>>> the specified memory
>>>> region has not been reserved using an existing mapping,
>>>> newer kernels
>>>> (Linux 4.17 and later) provide an option
>>>> MAP_FIXED_NOREPLACE that
>>>> should be used instead. Older kernels require the
>>>> caller to use addr as a hint and take appropriate action if
>>>> the kernel places the new mapping at a different address.
>>>>
>>>> John, Michael, what do you think?
>>>
>>>
>>> I'm still having difficulty with it, because this is in the MAP_FIXED section,
>>> but I think you're documenting the behavior that you get if you do *not*
>>> specify MAP_FIXED, right? Also, the hint behavior is true of both older and
>>> new kernels...
>>
>> The manpage patch you and mhocko wrote mentioned MAP_FIXED_NOREPLACE
>> in the MAP_FIXED section - I was trying to avoid undoing a change you
>> had just explicitly made.
>
> heh. So I've succeeding in getting my own wording removed, then? Progress! :)
>
>>
>>> So, if that's your intent (you want to sort of document by contrast to what
>>> would happen if this option were not used), then how about something like this:
>>>
>>>
>>> Without the MAP_FIXED option, the kernel would treat addr as a hint, rather
>>> than a requirement, and the caller would need to take appropriate action
>>> if the kernel placed the mapping at a different address. (For example,
>>> munmap and try again.)
>>
>> I'd be fine with removing the paragraph. As you rightly pointed out,
>> it doesn't really describe MAP_FIXED.
>>
>
> OK, that's probably the simplest fix.

So, you mean remove this entire paragraph:

For cases in which the specified memory region has not been
reserved using an existing mapping, newer kernels (Linux
4.17 and later) provide an option MAP_FIXED_NOREPLACE that
should be used instead; older kernels require the caller to
use addr as a hint (without MAP_FIXED) and take appropriate
action if the kernel places the new mapping at a different
address.

It seems like some version of the first half of the paragraph is worth
keeping, though, so as to point the reader in the direction of a remedy.
How about replacing that text with the following:

Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
in a multithreaded program to avoid the hazard described
above.

?

Thanks,

Michael

--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

2018-04-13 06:58:22

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
[...]
> So, you mean remove this entire paragraph:
>
> For cases in which the specified memory region has not been
> reserved using an existing mapping, newer kernels (Linux
> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
> should be used instead; older kernels require the caller to
> use addr as a hint (without MAP_FIXED) and take appropriate
> action if the kernel places the new mapping at a different
> address.
>
> It seems like some version of the first half of the paragraph is worth
> keeping, though, so as to point the reader in the direction of a remedy.
> How about replacing that text with the following:
>
> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
> in a multithreaded program to avoid the hazard described
> above.

Yes, that sounds reasonable to me.

--
Michal Hocko
SUSE Labs

2018-04-13 15:07:31

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <[email protected]> wrote:
> On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
> [...]
>> So, you mean remove this entire paragraph:
>>
>> For cases in which the specified memory region has not been
>> reserved using an existing mapping, newer kernels (Linux
>> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
>> should be used instead; older kernels require the caller to
>> use addr as a hint (without MAP_FIXED) and take appropriate
>> action if the kernel places the new mapping at a different
>> address.
>>
>> It seems like some version of the first half of the paragraph is worth
>> keeping, though, so as to point the reader in the direction of a remedy.
>> How about replacing that text with the following:
>>
>> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
>> in a multithreaded program to avoid the hazard described
>> above.
>
> Yes, that sounds reasonable to me.

But that kind of sounds as if you can't avoid it before Linux 4.17,
when actually, you just have to call mmap() with the address as hint,
and if mmap() returns a different address, munmap() it and go on your
normal error path.

2018-04-13 16:05:59

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Fri 13-04-18 17:04:09, Jann Horn wrote:
> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <[email protected]> wrote:
> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
> > [...]
> >> So, you mean remove this entire paragraph:
> >>
> >> For cases in which the specified memory region has not been
> >> reserved using an existing mapping, newer kernels (Linux
> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
> >> should be used instead; older kernels require the caller to
> >> use addr as a hint (without MAP_FIXED) and take appropriate
> >> action if the kernel places the new mapping at a different
> >> address.
> >>
> >> It seems like some version of the first half of the paragraph is worth
> >> keeping, though, so as to point the reader in the direction of a remedy.
> >> How about replacing that text with the following:
> >>
> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
> >> in a multithreaded program to avoid the hazard described
> >> above.
> >
> > Yes, that sounds reasonable to me.
>
> But that kind of sounds as if you can't avoid it before Linux 4.17,
> when actually, you just have to call mmap() with the address as hint,
> and if mmap() returns a different address, munmap() it and go on your
> normal error path.

This is still racy in multithreaded application which is the main point
of the whole section, no?

--
Michal Hocko
SUSE Labs

2018-04-13 16:08:31

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Fri, Apr 13, 2018 at 6:04 PM, Michal Hocko <[email protected]> wrote:
> On Fri 13-04-18 17:04:09, Jann Horn wrote:
>> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <[email protected]> wrote:
>> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
>> > [...]
>> >> So, you mean remove this entire paragraph:
>> >>
>> >> For cases in which the specified memory region has not been
>> >> reserved using an existing mapping, newer kernels (Linux
>> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
>> >> should be used instead; older kernels require the caller to
>> >> use addr as a hint (without MAP_FIXED) and take appropriate
>> >> action if the kernel places the new mapping at a different
>> >> address.
>> >>
>> >> It seems like some version of the first half of the paragraph is worth
>> >> keeping, though, so as to point the reader in the direction of a remedy.
>> >> How about replacing that text with the following:
>> >>
>> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
>> >> in a multithreaded program to avoid the hazard described
>> >> above.
>> >
>> > Yes, that sounds reasonable to me.
>>
>> But that kind of sounds as if you can't avoid it before Linux 4.17,
>> when actually, you just have to call mmap() with the address as hint,
>> and if mmap() returns a different address, munmap() it and go on your
>> normal error path.
>
> This is still racy in multithreaded application which is the main point
> of the whole section, no?

No, it isn't.

2018-04-13 16:19:17

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Fri, Apr 13, 2018 at 6:05 PM, Jann Horn <[email protected]> wrote:
> On Fri, Apr 13, 2018 at 6:04 PM, Michal Hocko <[email protected]> wrote:
>> On Fri 13-04-18 17:04:09, Jann Horn wrote:
>>> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <[email protected]> wrote:
>>> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
>>> > [...]
>>> >> So, you mean remove this entire paragraph:
>>> >>
>>> >> For cases in which the specified memory region has not been
>>> >> reserved using an existing mapping, newer kernels (Linux
>>> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
>>> >> should be used instead; older kernels require the caller to
>>> >> use addr as a hint (without MAP_FIXED) and take appropriate
>>> >> action if the kernel places the new mapping at a different
>>> >> address.
>>> >>
>>> >> It seems like some version of the first half of the paragraph is worth
>>> >> keeping, though, so as to point the reader in the direction of a remedy.
>>> >> How about replacing that text with the following:
>>> >>
>>> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
>>> >> in a multithreaded program to avoid the hazard described
>>> >> above.
>>> >
>>> > Yes, that sounds reasonable to me.
>>>
>>> But that kind of sounds as if you can't avoid it before Linux 4.17,
>>> when actually, you just have to call mmap() with the address as hint,
>>> and if mmap() returns a different address, munmap() it and go on your
>>> normal error path.
>>
>> This is still racy in multithreaded application which is the main point
>> of the whole section, no?
>
> No, it isn't.

mmap() with a hint (without MAP_FIXED) will always non-racily allocate
a memory region for you or return an error code. If it does allocate a
memory region, it belongs to you until you deallocate it. It might be
at a different address than you requested - in that case you can
emulate MAP_FIXED_NOREPLACE by calling munmap() and treating it as an
error; or you can do something else with it.

MAP_FIXED_NOREPLACE is just a performance optimization.

2018-04-16 10:09:28

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Fri 13-04-18 18:17:36, Jann Horn wrote:
> On Fri, Apr 13, 2018 at 6:05 PM, Jann Horn <[email protected]> wrote:
> > On Fri, Apr 13, 2018 at 6:04 PM, Michal Hocko <[email protected]> wrote:
> >> On Fri 13-04-18 17:04:09, Jann Horn wrote:
> >>> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <[email protected]> wrote:
> >>> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
> >>> > [...]
> >>> >> So, you mean remove this entire paragraph:
> >>> >>
> >>> >> For cases in which the specified memory region has not been
> >>> >> reserved using an existing mapping, newer kernels (Linux
> >>> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
> >>> >> should be used instead; older kernels require the caller to
> >>> >> use addr as a hint (without MAP_FIXED) and take appropriate
> >>> >> action if the kernel places the new mapping at a different
> >>> >> address.
> >>> >>
> >>> >> It seems like some version of the first half of the paragraph is worth
> >>> >> keeping, though, so as to point the reader in the direction of a remedy.
> >>> >> How about replacing that text with the following:
> >>> >>
> >>> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
> >>> >> in a multithreaded program to avoid the hazard described
> >>> >> above.
> >>> >
> >>> > Yes, that sounds reasonable to me.
> >>>
> >>> But that kind of sounds as if you can't avoid it before Linux 4.17,
> >>> when actually, you just have to call mmap() with the address as hint,
> >>> and if mmap() returns a different address, munmap() it and go on your
> >>> normal error path.
> >>
> >> This is still racy in multithreaded application which is the main point
> >> of the whole section, no?
> >
> > No, it isn't.

I could have been more specific, sorry.

> mmap() with a hint (without MAP_FIXED) will always non-racily allocate
> a memory region for you or return an error code. If it does allocate a
> memory region, it belongs to you until you deallocate it. It might be
> at a different address than you requested -

Yes, this all is true. Except the atomicity is guaranteed only for the
syscall. Once you return to the userspace any error handling is error
prone and racy because your mapping might change under you feet. So...

> in that case you can
> emulate MAP_FIXED_NOREPLACE by calling munmap() and treating it as an
> error; or you can do something else with it.
>
> MAP_FIXED_NOREPLACE is just a performance optimization.

This is not quite true because you get _your_ area or _an error_
atomically which is not possible with 2 syscalls.

--
Michal Hocko
SUSE Labs

2018-04-16 13:58:07

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Mon, Apr 16, 2018 at 12:07 PM, Michal Hocko <[email protected]> wrote:
> On Fri 13-04-18 18:17:36, Jann Horn wrote:
>> On Fri, Apr 13, 2018 at 6:05 PM, Jann Horn <[email protected]> wrote:
>> > On Fri, Apr 13, 2018 at 6:04 PM, Michal Hocko <[email protected]> wrote:
>> >> On Fri 13-04-18 17:04:09, Jann Horn wrote:
>> >>> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <[email protected]> wrote:
>> >>> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
>> >>> > [...]
>> >>> >> So, you mean remove this entire paragraph:
>> >>> >>
>> >>> >> For cases in which the specified memory region has not been
>> >>> >> reserved using an existing mapping, newer kernels (Linux
>> >>> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
>> >>> >> should be used instead; older kernels require the caller to
>> >>> >> use addr as a hint (without MAP_FIXED) and take appropriate
>> >>> >> action if the kernel places the new mapping at a different
>> >>> >> address.
>> >>> >>
>> >>> >> It seems like some version of the first half of the paragraph is worth
>> >>> >> keeping, though, so as to point the reader in the direction of a remedy.
>> >>> >> How about replacing that text with the following:
>> >>> >>
>> >>> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
>> >>> >> in a multithreaded program to avoid the hazard described
>> >>> >> above.
>> >>> >
>> >>> > Yes, that sounds reasonable to me.
>> >>>
>> >>> But that kind of sounds as if you can't avoid it before Linux 4.17,
>> >>> when actually, you just have to call mmap() with the address as hint,
>> >>> and if mmap() returns a different address, munmap() it and go on your
>> >>> normal error path.
>> >>
>> >> This is still racy in multithreaded application which is the main point
>> >> of the whole section, no?
>> >
>> > No, it isn't.
>
> I could have been more specific, sorry.
>
>> mmap() with a hint (without MAP_FIXED) will always non-racily allocate
>> a memory region for you or return an error code. If it does allocate a
>> memory region, it belongs to you until you deallocate it. It might be
>> at a different address than you requested -
>
> Yes, this all is true. Except the atomicity is guaranteed only for the
> syscall. Once you return to the userspace any error handling is error
> prone and racy because your mapping might change under you feet. So...

Can you please elaborate on why you think anything could change the
mapping returned by mmap() under the caller's feet?
When mmap() returns a memory area to the caller, that memory area
belongs to the caller. No unrelated code will touch it, unless that
code is buggy.

>> in that case you can
>> emulate MAP_FIXED_NOREPLACE by calling munmap() and treating it as an
>> error; or you can do something else with it.
>>
>> MAP_FIXED_NOREPLACE is just a performance optimization.
>
> This is not quite true because you get _your_ area or _an error_
> atomically which is not possible with 2 syscalls.

Why not?

2018-04-16 19:19:36

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Mon 16-04-18 15:55:36, Jann Horn wrote:
> On Mon, Apr 16, 2018 at 12:07 PM, Michal Hocko <[email protected]> wrote:
> > On Fri 13-04-18 18:17:36, Jann Horn wrote:
> >> On Fri, Apr 13, 2018 at 6:05 PM, Jann Horn <[email protected]> wrote:
> >> > On Fri, Apr 13, 2018 at 6:04 PM, Michal Hocko <[email protected]> wrote:
> >> >> On Fri 13-04-18 17:04:09, Jann Horn wrote:
> >> >>> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <[email protected]> wrote:
> >> >>> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
> >> >>> > [...]
> >> >>> >> So, you mean remove this entire paragraph:
> >> >>> >>
> >> >>> >> For cases in which the specified memory region has not been
> >> >>> >> reserved using an existing mapping, newer kernels (Linux
> >> >>> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
> >> >>> >> should be used instead; older kernels require the caller to
> >> >>> >> use addr as a hint (without MAP_FIXED) and take appropriate
> >> >>> >> action if the kernel places the new mapping at a different
> >> >>> >> address.
> >> >>> >>
> >> >>> >> It seems like some version of the first half of the paragraph is worth
> >> >>> >> keeping, though, so as to point the reader in the direction of a remedy.
> >> >>> >> How about replacing that text with the following:
> >> >>> >>
> >> >>> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
> >> >>> >> in a multithreaded program to avoid the hazard described
> >> >>> >> above.
> >> >>> >
> >> >>> > Yes, that sounds reasonable to me.
> >> >>>
> >> >>> But that kind of sounds as if you can't avoid it before Linux 4.17,
> >> >>> when actually, you just have to call mmap() with the address as hint,
> >> >>> and if mmap() returns a different address, munmap() it and go on your
> >> >>> normal error path.
> >> >>
> >> >> This is still racy in multithreaded application which is the main point
> >> >> of the whole section, no?
> >> >
> >> > No, it isn't.
> >
> > I could have been more specific, sorry.
> >
> >> mmap() with a hint (without MAP_FIXED) will always non-racily allocate
> >> a memory region for you or return an error code. If it does allocate a
> >> memory region, it belongs to you until you deallocate it. It might be
> >> at a different address than you requested -
> >
> > Yes, this all is true. Except the atomicity is guaranteed only for the
> > syscall. Once you return to the userspace any error handling is error
> > prone and racy because your mapping might change under you feet. So...
>
> Can you please elaborate on why you think anything could change the
> mapping returned by mmap() under the caller's feet?

Because as soon as the mmap_sem is dropped then any other thread can
modify the shared address space.

> When mmap() returns a memory area to the caller, that memory area
> belongs to the caller. No unrelated code will touch it, unless that
> code is buggy.

Yes, reasonably well written application will not have this problem.
That, however, requires an external synchronization and that's why
called it error prone and racy. I guess that was the main motivation for
that part of the man page.
--
Michal Hocko
SUSE Labs

2018-04-16 19:32:21

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Mon, Apr 16, 2018 at 9:18 PM, Michal Hocko <[email protected]> wrote:
> On Mon 16-04-18 15:55:36, Jann Horn wrote:
>> On Mon, Apr 16, 2018 at 12:07 PM, Michal Hocko <[email protected]> wrote:
>> > On Fri 13-04-18 18:17:36, Jann Horn wrote:
>> >> On Fri, Apr 13, 2018 at 6:05 PM, Jann Horn <[email protected]> wrote:
>> >> > On Fri, Apr 13, 2018 at 6:04 PM, Michal Hocko <[email protected]> wrote:
>> >> >> On Fri 13-04-18 17:04:09, Jann Horn wrote:
>> >> >>> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <[email protected]> wrote:
>> >> >>> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
>> >> >>> > [...]
>> >> >>> >> So, you mean remove this entire paragraph:
>> >> >>> >>
>> >> >>> >> For cases in which the specified memory region has not been
>> >> >>> >> reserved using an existing mapping, newer kernels (Linux
>> >> >>> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
>> >> >>> >> should be used instead; older kernels require the caller to
>> >> >>> >> use addr as a hint (without MAP_FIXED) and take appropriate
>> >> >>> >> action if the kernel places the new mapping at a different
>> >> >>> >> address.
>> >> >>> >>
>> >> >>> >> It seems like some version of the first half of the paragraph is worth
>> >> >>> >> keeping, though, so as to point the reader in the direction of a remedy.
>> >> >>> >> How about replacing that text with the following:
>> >> >>> >>
>> >> >>> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
>> >> >>> >> in a multithreaded program to avoid the hazard described
>> >> >>> >> above.
>> >> >>> >
>> >> >>> > Yes, that sounds reasonable to me.
>> >> >>>
>> >> >>> But that kind of sounds as if you can't avoid it before Linux 4.17,
>> >> >>> when actually, you just have to call mmap() with the address as hint,
>> >> >>> and if mmap() returns a different address, munmap() it and go on your
>> >> >>> normal error path.
>> >> >>
>> >> >> This is still racy in multithreaded application which is the main point
>> >> >> of the whole section, no?
>> >> >
>> >> > No, it isn't.
>> >
>> > I could have been more specific, sorry.
>> >
>> >> mmap() with a hint (without MAP_FIXED) will always non-racily allocate
>> >> a memory region for you or return an error code. If it does allocate a
>> >> memory region, it belongs to you until you deallocate it. It might be
>> >> at a different address than you requested -
>> >
>> > Yes, this all is true. Except the atomicity is guaranteed only for the
>> > syscall. Once you return to the userspace any error handling is error
>> > prone and racy because your mapping might change under you feet. So...
>>
>> Can you please elaborate on why you think anything could change the
>> mapping returned by mmap() under the caller's feet?
>
> Because as soon as the mmap_sem is dropped then any other thread can
> modify the shared address space.
>
>> When mmap() returns a memory area to the caller, that memory area
>> belongs to the caller. No unrelated code will touch it, unless that
>> code is buggy.
>
> Yes, reasonably well written application will not have this problem.
> That, however, requires an external synchronization and that's why
> called it error prone and racy. I guess that was the main motivation for
> that part of the man page.

What requires external synchronization? I still don't understand at
all what you're talking about.

The following code:

void *try_to_alloc_addr(void *hint, size_t len) {
char *x = mmap(hint, len, ...);
if (x == MAP_FAILED) return NULL;
if (x == hint) return x;
munmap(x, len);
return NULL;
}

has no need for any form of external synchronization. You can call it
in library code, you can call it in a multithreaded process, you can
call it wherever and it should be safe.

mmap() atomically reserves previously unallocated memory, and nothing
else should be touching that memory until it is released again using
munmap(). (Just like malloc(): When you call malloc(), you get a chunk
of memory that is reserved just for you, and nobody else will scribble
over it until you call free().)

2018-04-16 19:58:51

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Mon 16-04-18 21:30:09, Jann Horn wrote:
> On Mon, Apr 16, 2018 at 9:18 PM, Michal Hocko <[email protected]> wrote:
[...]
> > Yes, reasonably well written application will not have this problem.
> > That, however, requires an external synchronization and that's why
> > called it error prone and racy. I guess that was the main motivation for
> > that part of the man page.
>
> What requires external synchronization? I still don't understand at
> all what you're talking about.
>
> The following code:
>
> void *try_to_alloc_addr(void *hint, size_t len) {
> char *x = mmap(hint, len, ...);
> if (x == MAP_FAILED) return NULL;
> if (x == hint) return x;

Any other thread can modify the address space at this moment. Just
consider that another thread would does mmap(x, MAP_FIXED) (or any other
address overlapping [x, x+len] range) becaus it is seemingly safe as x
!= hint. This will succeed and ...
> munmap(x, len);
... now you are munmaping somebody's else memory range

> return NULL;

Do code _is_ buggy but it is not obvious at all.

> }
>
> has no need for any form of external synchronization.

If the above mmap/munmap section was protected by a lock and _all_ other
mmaps (direct or indirect) would use the same lock then you are safe
against that.
--
Michal Hocko
SUSE Labs

2018-04-16 20:21:18

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Mon, Apr 16, 2018 at 9:57 PM, Michal Hocko <[email protected]> wrote:
> On Mon 16-04-18 21:30:09, Jann Horn wrote:
>> On Mon, Apr 16, 2018 at 9:18 PM, Michal Hocko <[email protected]> wrote:
> [...]
>> > Yes, reasonably well written application will not have this problem.
>> > That, however, requires an external synchronization and that's why
>> > called it error prone and racy. I guess that was the main motivation for
>> > that part of the man page.
>>
>> What requires external synchronization? I still don't understand at
>> all what you're talking about.
>>
>> The following code:
>>
>> void *try_to_alloc_addr(void *hint, size_t len) {
>> char *x = mmap(hint, len, ...);
>> if (x == MAP_FAILED) return NULL;
>> if (x == hint) return x;
>
> Any other thread can modify the address space at this moment.

But not parts of the address space that were returned by this mmap() call.

> Just
> consider that another thread would does mmap(x, MAP_FIXED) (or any other
> address overlapping [x, x+len] range)

If the other thread does that without previously having created a
mapping covering the area in question, that would be a bug in the
other thread. MAP_FIXED on an unmapped address is almost always a bug
(excluding single-threaded cases with no library code, and even then
it's quite weird) - for example, any malloc() call could also cause
libc to start using the memory range you're trying to map with
MAP_FIXED.

> becaus it is seemingly safe as x
> != hint.

I don't understand this part. Are you talking about a hypothetical
scenario in which a programmer attempts to segment the virtual memory
space into areas that are exclusively used by threads without creating
memory mappings for those areas?

> This will succeed and ...
>> munmap(x, len);
> ... now you are munmaping somebody's else memory range
>
>> return NULL;
>
> Do code _is_ buggy but it is not obvious at all.
>
>> }
>>
>> has no need for any form of external synchronization.
>
> If the above mmap/munmap section was protected by a lock and _all_ other
> mmaps (direct or indirect) would use the same lock then you are safe
> against that.

2018-04-16 21:13:02

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Mon 16-04-18 22:17:40, Jann Horn wrote:
> On Mon, Apr 16, 2018 at 9:57 PM, Michal Hocko <[email protected]> wrote:
> > On Mon 16-04-18 21:30:09, Jann Horn wrote:
> >> On Mon, Apr 16, 2018 at 9:18 PM, Michal Hocko <[email protected]> wrote:
> > [...]
> >> > Yes, reasonably well written application will not have this problem.
> >> > That, however, requires an external synchronization and that's why
> >> > called it error prone and racy. I guess that was the main motivation for
> >> > that part of the man page.
> >>
> >> What requires external synchronization? I still don't understand at
> >> all what you're talking about.
> >>
> >> The following code:
> >>
> >> void *try_to_alloc_addr(void *hint, size_t len) {
> >> char *x = mmap(hint, len, ...);
> >> if (x == MAP_FAILED) return NULL;
> >> if (x == hint) return x;
> >
> > Any other thread can modify the address space at this moment.
>
> But not parts of the address space that were returned by this mmap() call.
?
> > Just
> > consider that another thread would does mmap(x, MAP_FIXED) (or any other
> > address overlapping [x, x+len] range)
>
> If the other thread does that without previously having created a
> mapping covering the area in question, that would be a bug in the
> other thread.

MAP_FIXED is sometimes used without preallocated address ranges.

> MAP_FIXED on an unmapped address is almost always a bug
> (excluding single-threaded cases with no library code, and even then
> it's quite weird) - for example, any malloc() call could also cause
> libc to start using the memory range you're trying to map with
> MAP_FIXED.

Yeah and that's why we there is such a large paragraph in the man page
;)

> > becaus it is seemingly safe as x
> > != hint.
>
> I don't understand this part. Are you talking about a hypothetical
> scenario in which a programmer attempts to segment the virtual memory
> space into areas that are exclusively used by threads without creating
> memory mappings for those areas?

Yeah, that doesn't sound all that over-exaggerated, right? And yes,
such a code would be subtle and most probably buggy. I am not trying to
argue for those hypothetical cases. All I am saying is that MAP_FIXED is
subtle.

I _do_ agree that using it solely on the preallocated and _properly_
managed address ranges is safe. I still maintain my position on error
prone though. And besides that there are usecases which do not operate
on preallocated address ranges so people really have to be careful.

I do not really care what is the form. I find the current wording quite
informative and showing examples of how things might be broken. I do
agree with your remark that "MAP_FIXED on preallocated ranges is safe"
should be added. But MAP_FIXED is dangerous API and should have few big
fat warnings.
--
Michal Hocko
SUSE Labs

2018-04-16 21:15:47

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Mon, Apr 16, 2018 at 11:11 PM, Michal Hocko <[email protected]> wrote:
> On Mon 16-04-18 22:17:40, Jann Horn wrote:
>> On Mon, Apr 16, 2018 at 9:57 PM, Michal Hocko <[email protected]> wrote:
>> > On Mon 16-04-18 21:30:09, Jann Horn wrote:
>> >> On Mon, Apr 16, 2018 at 9:18 PM, Michal Hocko <[email protected]> wrote:
>> > [...]
>> >> > Yes, reasonably well written application will not have this problem.
>> >> > That, however, requires an external synchronization and that's why
>> >> > called it error prone and racy. I guess that was the main motivation for
>> >> > that part of the man page.
>> >>
>> >> What requires external synchronization? I still don't understand at
>> >> all what you're talking about.
>> >>
>> >> The following code:
>> >>
>> >> void *try_to_alloc_addr(void *hint, size_t len) {
>> >> char *x = mmap(hint, len, ...);
>> >> if (x == MAP_FAILED) return NULL;
>> >> if (x == hint) return x;
>> >
>> > Any other thread can modify the address space at this moment.
>>
>> But not parts of the address space that were returned by this mmap() call.
> ?
>> > Just
>> > consider that another thread would does mmap(x, MAP_FIXED) (or any other
>> > address overlapping [x, x+len] range)
>>
>> If the other thread does that without previously having created a
>> mapping covering the area in question, that would be a bug in the
>> other thread.
>
> MAP_FIXED is sometimes used without preallocated address ranges.

Wow, really? Can you point to an example?

>> MAP_FIXED on an unmapped address is almost always a bug
>> (excluding single-threaded cases with no library code, and even then
>> it's quite weird) - for example, any malloc() call could also cause
>> libc to start using the memory range you're trying to map with
>> MAP_FIXED.
>
> Yeah and that's why we there is such a large paragraph in the man page
> ;)
>
>> > becaus it is seemingly safe as x
>> > != hint.
>>
>> I don't understand this part. Are you talking about a hypothetical
>> scenario in which a programmer attempts to segment the virtual memory
>> space into areas that are exclusively used by threads without creating
>> memory mappings for those areas?
>
> Yeah, that doesn't sound all that over-exaggerated, right? And yes,
> such a code would be subtle and most probably buggy. I am not trying to
> argue for those hypothetical cases. All I am saying is that MAP_FIXED is
> subtle.
>
> I _do_ agree that using it solely on the preallocated and _properly_
> managed address ranges is safe. I still maintain my position on error
> prone though. And besides that there are usecases which do not operate
> on preallocated address ranges so people really have to be careful.
>
> I do not really care what is the form. I find the current wording quite
> informative and showing examples of how things might be broken. I do
> agree with your remark that "MAP_FIXED on preallocated ranges is safe"
> should be added. But MAP_FIXED is dangerous API and should have few big
> fat warnings.
> --
> Michal Hocko
> SUSE Labs

2018-04-17 06:24:38

[permalink] [raw]

Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved

On Mon 16-04-18 23:12:48, Jann Horn wrote:
> On Mon, Apr 16, 2018 at 11:11 PM, Michal Hocko <[email protected]> wrote:
> > On Mon 16-04-18 22:17:40, Jann Horn wrote:
> >> On Mon, Apr 16, 2018 at 9:57 PM, Michal Hocko <[email protected]> wrote:
> >> > On Mon 16-04-18 21:30:09, Jann Horn wrote:
> >> >> On Mon, Apr 16, 2018 at 9:18 PM, Michal Hocko <[email protected]> wrote:
> >> > [...]
> >> >> > Yes, reasonably well written application will not have this problem.
> >> >> > That, however, requires an external synchronization and that's why
> >> >> > called it error prone and racy. I guess that was the main motivation for
> >> >> > that part of the man page.
> >> >>
> >> >> What requires external synchronization? I still don't understand at
> >> >> all what you're talking about.
> >> >>
> >> >> The following code:
> >> >>
> >> >> void *try_to_alloc_addr(void *hint, size_t len) {
> >> >> char *x = mmap(hint, len, ...);
> >> >> if (x == MAP_FAILED) return NULL;
> >> >> if (x == hint) return x;
> >> >
> >> > Any other thread can modify the address space at this moment.
> >>
> >> But not parts of the address space that were returned by this mmap() call.
> > ?
> >> > Just
> >> > consider that another thread would does mmap(x, MAP_FIXED) (or any other
> >> > address overlapping [x, x+len] range)
> >>
> >> If the other thread does that without previously having created a
> >> mapping covering the area in question, that would be a bug in the
> >> other thread.
> >
> > MAP_FIXED is sometimes used without preallocated address ranges.
>
> Wow, really? Can you point to an example?

Just from top of my head.

Some of that is for historical reasons because the hint address used to
be ignored on some operating systems so MAP_FIXED had to be used.

Currently not user I guess but MAP_FIXED for addresses above 47b address
space AFAIR.

And I am pretty sure there would be much more if you actually browsed
code search.
--
Michal Hocko
SUSE Labs

2018-05-02 13:07:25