Re: [PATCH v5] mmap.2: MAP_FIXED updated documentation

2017-12-18 Thread Michael Kerrisk (man-pages) 
Hi John,

On 12/18/2017 10:27 PM, John Hubbard wrote:
> On 12/18/2017 11:15 AM, Michael Kerrisk (man-pages) wrote:
>> On 12/12/2017 01:23 AM, john.hubb...@gmail.com wrote:
>>> From: John Hubbard 
>>>
>>> -- Expand the documentation to discuss the hazards in
>>>enough detail to allow avoiding them.
>>>
>>> -- Mention the upcoming MAP_FIXED_SAFE flag.
>>>
>>> -- Enhance the alignment requirement slightly.
>>>
>>> CC: Michael Ellerman 
>>> CC: Jann Horn 
>>> CC: Matthew Wilcox 
>>> CC: Michal Hocko 
>>> CC: Mike Rapoport 
>>> CC: Cyril Hrubis 
>>> CC: Michal Hocko 
>>> CC: Pavel Machek 
>>> Signed-off-by: John Hubbard 
>>
>> John,
>>
>> Thanks for the patch. I think you win the prize for the 
>> most iterations ever on a man-pages patch! (And Michal,
>> thanks for helping out.) I've applied your patch, made 
>> some minor tweaks, and removed the mention of 
>> MAP_FIXED_SAFE, since I don't like to document stuff
>> that hasn't yet been merged. (I only later noticed the
>> fuss about the naming...)
>>
> 
> Hi Michael,
> 
> The final result looks nice, thanks for all the editing fixes.
> 
> One last thing: reading through this, I think it might need a wording
> fix (this is my fault), in order to avoid implying that brk() or
> malloc() use dlopen().
> 
> Something approximately like this:
> 
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index 79681b31e..1c0bd80de 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -250,8 +250,9 @@ suffice.
>  The
>  .BR dlopen (3)
>  call will map the library into the process's address space.
> -Furthermore, almost any library call may be implemented using this technique.
> -Examples include
> +Furthermore, almost any library call may be implemented in a way that
> +adds memory mappings to the address space, either with this technique,
> +or by simply allocating memory. Examples include
>  .BR brk (2),
>  .BR malloc (3),
>  .BR pthread_create (3),
> 
> 
> ...or does the current version seem OK to other people?

Thanks. Looks good to me. 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/

Re: [PATCH v5] mmap.2: MAP_FIXED updated documentation

2017-12-18 Thread Michael Kerrisk (man-pages) 
Hi John,

On 12/18/2017 10:27 PM, John Hubbard wrote:
> On 12/18/2017 11:15 AM, Michael Kerrisk (man-pages) wrote:
>> On 12/12/2017 01:23 AM, john.hubb...@gmail.com wrote:
>>> From: John Hubbard 
>>>
>>> -- Expand the documentation to discuss the hazards in
>>>enough detail to allow avoiding them.
>>>
>>> -- Mention the upcoming MAP_FIXED_SAFE flag.
>>>
>>> -- Enhance the alignment requirement slightly.
>>>
>>> CC: Michael Ellerman 
>>> CC: Jann Horn 
>>> CC: Matthew Wilcox 
>>> CC: Michal Hocko 
>>> CC: Mike Rapoport 
>>> CC: Cyril Hrubis 
>>> CC: Michal Hocko 
>>> CC: Pavel Machek 
>>> Signed-off-by: John Hubbard 
>>
>> John,
>>
>> Thanks for the patch. I think you win the prize for the 
>> most iterations ever on a man-pages patch! (And Michal,
>> thanks for helping out.) I've applied your patch, made 
>> some minor tweaks, and removed the mention of 
>> MAP_FIXED_SAFE, since I don't like to document stuff
>> that hasn't yet been merged. (I only later noticed the
>> fuss about the naming...)
>>
> 
> Hi Michael,
> 
> The final result looks nice, thanks for all the editing fixes.
> 
> One last thing: reading through this, I think it might need a wording
> fix (this is my fault), in order to avoid implying that brk() or
> malloc() use dlopen().
> 
> Something approximately like this:
> 
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index 79681b31e..1c0bd80de 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -250,8 +250,9 @@ suffice.
>  The
>  .BR dlopen (3)
>  call will map the library into the process's address space.
> -Furthermore, almost any library call may be implemented using this technique.
> -Examples include
> +Furthermore, almost any library call may be implemented in a way that
> +adds memory mappings to the address space, either with this technique,
> +or by simply allocating memory. Examples include
>  .BR brk (2),
>  .BR malloc (3),
>  .BR pthread_create (3),
> 
> 
> ...or does the current version seem OK to other people?

Thanks. Looks good to me. 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/

Re: [PATCH v5] mmap.2: MAP_FIXED updated documentation

2017-12-18 Thread John Hubbard 
On 12/18/2017 11:15 AM, Michael Kerrisk (man-pages) wrote:
> On 12/12/2017 01:23 AM, john.hubb...@gmail.com wrote:
>> From: John Hubbard 
>>
>> -- Expand the documentation to discuss the hazards in
>>enough detail to allow avoiding them.
>>
>> -- Mention the upcoming MAP_FIXED_SAFE flag.
>>
>> -- Enhance the alignment requirement slightly.
>>
>> CC: Michael Ellerman 
>> CC: Jann Horn 
>> CC: Matthew Wilcox 
>> CC: Michal Hocko 
>> CC: Mike Rapoport 
>> CC: Cyril Hrubis 
>> CC: Michal Hocko 
>> CC: Pavel Machek 
>> Signed-off-by: John Hubbard 
> 
> John,
> 
> Thanks for the patch. I think you win the prize for the 
> most iterations ever on a man-pages patch! (And Michal,
> thanks for helping out.) I've applied your patch, made 
> some minor tweaks, and removed the mention of 
> MAP_FIXED_SAFE, since I don't like to document stuff
> that hasn't yet been merged. (I only later noticed the
> fuss about the naming...)
> 

Hi Michael,

The final result looks nice, thanks for all the editing fixes.

One last thing: reading through this, I think it might need a wording
fix (this is my fault), in order to avoid implying that brk() or
malloc() use dlopen().

Something approximately like this:

diff --git a/man2/mmap.2 b/man2/mmap.2
index 79681b31e..1c0bd80de 100644
--- a/man2/mmap.2
+++ b/man2/mmap.2
@@ -250,8 +250,9 @@ suffice.
 The
 .BR dlopen (3)
 call will map the library into the process's address space.
-Furthermore, almost any library call may be implemented using this technique.
-Examples include
+Furthermore, almost any library call may be implemented in a way that
+adds memory mappings to the address space, either with this technique,
+or by simply allocating memory. Examples include
 .BR brk (2),
 .BR malloc (3),
 .BR pthread_create (3),


...or does the current version seem OK to other people?

thanks,
-- 
John Hubbard
NVIDIA

> Cheers,
> 
> Michael
> 
>> ---
>>
>> Changes since v4:
>>
>> -- v2 ("mmap.2: MAP_FIXED is no longer discouraged") was applied already,
>>so v5 is a merge, including rewording of the paragraph transitions.
>>
>> -- We seem to have consensus about what to say about alignment
>>now, and this includes that new wording.
>>
>> Changes since v3:
>>
>> -- Removed the "how to use this safely" part, and
>>the SHMLBA part, both as a result of Michal Hocko's
>>review.
>>
>> -- A few tiny wording fixes, at the not-quite-typo level.
>>
>> Changes since v2:
>>
>> -- Fixed up the "how to use safely" example, in response
>>to Mike Rapoport's review.
>>
>> -- Changed the alignment requirement from system page
>>size, to SHMLBA. This was inspired by (but not yet
>>recommended by) Cyril Hrubis' review.
>>
>> -- Formatting: underlined /proc//maps
>>
>> Changes since v1:
>>
>> -- Covered topics recommended by Matthew Wilcox
>>and Jann Horn, in their recent review: the hazards
>>of overwriting pre-exising mappings, and some notes
>>about how to use MAP_FIXED safely.
>>
>> -- Rewrote the commit description accordingly.
>>
>>  man2/mmap.2 | 32 ++--
>>  1 file changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/man2/mmap.2 b/man2/mmap.2
>> index a5a8eb47a..400cfda2d 100644
>> --- a/man2/mmap.2
>> +++ b/man2/mmap.2
>> @@ -212,8 +212,9 @@ Don't interpret
>>  .I addr
>>  as a hint: place the mapping at exactly that address.
>>  .I addr
>> -must be a multiple of the page size.
>> -If the memory region specified by
>> +must be suitably aligned: for most architectures a multiple of page
>> +size is sufficient; however, some architectures may impose additional
>> +restrictions. If the memory region specified by
>>  .I addr
>>  and
>>  .I len
>> @@ -226,6 +227,33 @@ Software that aspires to be portable should use this 
>> option with care, keeping
>>  in mind that the exact layout of a process' memory map is allowed to change
>>  significantly between kernel versions, C library versions, and operating 
>> system
>>  releases.
>> +.IP
>> +Furthermore, this option is extremely hazardous (when used on its own), 
>> because
>> +it forcibly removes pre-existing mappings, making it easy for a 
>> multi-threaded
>> +process to corrupt its own address space.
>> +.IP
>> +For example, thread A looks through
>> +.I /proc//maps
>> +and locates an available
>> +address range, while thread B simultaneously acquires part or all of that 
>> same
>> +address range. Thread A then calls mmap(MAP_FIXED), effectively overwriting
>> +the mapping that thread B created.
>> +.IP
>> +Thread B need not create a mapping directly; simply making a library call
>> +that, internally, uses
>> +.I dlopen(3)
>> +to load some other shared library, will
>> +suffice. The dlopen(3) call

Re: [PATCH v5] mmap.2: MAP_FIXED updated documentation

2017-12-18 Thread John Hubbard 
On 12/18/2017 11:15 AM, Michael Kerrisk (man-pages) wrote:
> On 12/12/2017 01:23 AM, john.hubb...@gmail.com wrote:
>> From: John Hubbard 
>>
>> -- Expand the documentation to discuss the hazards in
>>enough detail to allow avoiding them.
>>
>> -- Mention the upcoming MAP_FIXED_SAFE flag.
>>
>> -- Enhance the alignment requirement slightly.
>>
>> CC: Michael Ellerman 
>> CC: Jann Horn 
>> CC: Matthew Wilcox 
>> CC: Michal Hocko 
>> CC: Mike Rapoport 
>> CC: Cyril Hrubis 
>> CC: Michal Hocko 
>> CC: Pavel Machek 
>> Signed-off-by: John Hubbard 
> 
> John,
> 
> Thanks for the patch. I think you win the prize for the 
> most iterations ever on a man-pages patch! (And Michal,
> thanks for helping out.) I've applied your patch, made 
> some minor tweaks, and removed the mention of 
> MAP_FIXED_SAFE, since I don't like to document stuff
> that hasn't yet been merged. (I only later noticed the
> fuss about the naming...)
> 

Hi Michael,

The final result looks nice, thanks for all the editing fixes.

One last thing: reading through this, I think it might need a wording
fix (this is my fault), in order to avoid implying that brk() or
malloc() use dlopen().

Something approximately like this:

diff --git a/man2/mmap.2 b/man2/mmap.2
index 79681b31e..1c0bd80de 100644
--- a/man2/mmap.2
+++ b/man2/mmap.2
@@ -250,8 +250,9 @@ suffice.
 The
 .BR dlopen (3)
 call will map the library into the process's address space.
-Furthermore, almost any library call may be implemented using this technique.
-Examples include
+Furthermore, almost any library call may be implemented in a way that
+adds memory mappings to the address space, either with this technique,
+or by simply allocating memory. Examples include
 .BR brk (2),
 .BR malloc (3),
 .BR pthread_create (3),


...or does the current version seem OK to other people?

thanks,
-- 
John Hubbard
NVIDIA

> Cheers,
> 
> Michael
> 
>> ---
>>
>> Changes since v4:
>>
>> -- v2 ("mmap.2: MAP_FIXED is no longer discouraged") was applied already,
>>so v5 is a merge, including rewording of the paragraph transitions.
>>
>> -- We seem to have consensus about what to say about alignment
>>now, and this includes that new wording.
>>
>> Changes since v3:
>>
>> -- Removed the "how to use this safely" part, and
>>the SHMLBA part, both as a result of Michal Hocko's
>>review.
>>
>> -- A few tiny wording fixes, at the not-quite-typo level.
>>
>> Changes since v2:
>>
>> -- Fixed up the "how to use safely" example, in response
>>to Mike Rapoport's review.
>>
>> -- Changed the alignment requirement from system page
>>size, to SHMLBA. This was inspired by (but not yet
>>recommended by) Cyril Hrubis' review.
>>
>> -- Formatting: underlined /proc//maps
>>
>> Changes since v1:
>>
>> -- Covered topics recommended by Matthew Wilcox
>>and Jann Horn, in their recent review: the hazards
>>of overwriting pre-exising mappings, and some notes
>>about how to use MAP_FIXED safely.
>>
>> -- Rewrote the commit description accordingly.
>>
>>  man2/mmap.2 | 32 ++--
>>  1 file changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/man2/mmap.2 b/man2/mmap.2
>> index a5a8eb47a..400cfda2d 100644
>> --- a/man2/mmap.2
>> +++ b/man2/mmap.2
>> @@ -212,8 +212,9 @@ Don't interpret
>>  .I addr
>>  as a hint: place the mapping at exactly that address.
>>  .I addr
>> -must be a multiple of the page size.
>> -If the memory region specified by
>> +must be suitably aligned: for most architectures a multiple of page
>> +size is sufficient; however, some architectures may impose additional
>> +restrictions. If the memory region specified by
>>  .I addr
>>  and
>>  .I len
>> @@ -226,6 +227,33 @@ Software that aspires to be portable should use this 
>> option with care, keeping
>>  in mind that the exact layout of a process' memory map is allowed to change
>>  significantly between kernel versions, C library versions, and operating 
>> system
>>  releases.
>> +.IP
>> +Furthermore, this option is extremely hazardous (when used on its own), 
>> because
>> +it forcibly removes pre-existing mappings, making it easy for a 
>> multi-threaded
>> +process to corrupt its own address space.
>> +.IP
>> +For example, thread A looks through
>> +.I /proc//maps
>> +and locates an available
>> +address range, while thread B simultaneously acquires part or all of that 
>> same
>> +address range. Thread A then calls mmap(MAP_FIXED), effectively overwriting
>> +the mapping that thread B created.
>> +.IP
>> +Thread B need not create a mapping directly; simply making a library call
>> +that, internally, uses
>> +.I dlopen(3)
>> +to load some other shared library, will
>> +suffice. The dlopen(3) call will map the library into the process's address
>> +space. Furthermore, almost any library call may be implemented using this
>> +technique.
>> +Examples include brk(2), malloc(3),

Re: [PATCH v5] mmap.2: MAP_FIXED updated documentation

2017-12-18 Thread Michael Kerrisk (man-pages) 
On 12/12/2017 01:23 AM, john.hubb...@gmail.com wrote:
> From: John Hubbard 
> 
> -- Expand the documentation to discuss the hazards in
>enough detail to allow avoiding them.
> 
> -- Mention the upcoming MAP_FIXED_SAFE flag.
> 
> -- Enhance the alignment requirement slightly.
> 
> CC: Michael Ellerman 
> CC: Jann Horn 
> CC: Matthew Wilcox 
> CC: Michal Hocko 
> CC: Mike Rapoport 
> CC: Cyril Hrubis 
> CC: Michal Hocko 
> CC: Pavel Machek 
> Signed-off-by: John Hubbard 

John,

Thanks for the patch. I think you win the prize for the 
most iterations ever on a man-pages patch! (And Michal,
thanks for helping out.) I've applied your patch, made 
some minor tweaks, and removed the mention of 
MAP_FIXED_SAFE, since I don't like to document stuff
that hasn't yet been merged. (I only later noticed the
fuss about the naming...)

Cheers,

Michael

> ---
> 
> Changes since v4:
> 
> -- v2 ("mmap.2: MAP_FIXED is no longer discouraged") was applied already,
>so v5 is a merge, including rewording of the paragraph transitions.
> 
> -- We seem to have consensus about what to say about alignment
>now, and this includes that new wording.
> 
> Changes since v3:
> 
> -- Removed the "how to use this safely" part, and
>the SHMLBA part, both as a result of Michal Hocko's
>review.
> 
> -- A few tiny wording fixes, at the not-quite-typo level.
> 
> Changes since v2:
> 
> -- Fixed up the "how to use safely" example, in response
>to Mike Rapoport's review.
> 
> -- Changed the alignment requirement from system page
>size, to SHMLBA. This was inspired by (but not yet
>recommended by) Cyril Hrubis' review.
> 
> -- Formatting: underlined /proc//maps
> 
> Changes since v1:
> 
> -- Covered topics recommended by Matthew Wilcox
>and Jann Horn, in their recent review: the hazards
>of overwriting pre-exising mappings, and some notes
>about how to use MAP_FIXED safely.
> 
> -- Rewrote the commit description accordingly.
> 
>  man2/mmap.2 | 32 ++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index a5a8eb47a..400cfda2d 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -212,8 +212,9 @@ Don't interpret
>  .I addr
>  as a hint: place the mapping at exactly that address.
>  .I addr
> -must be a multiple of the page size.
> -If the memory region specified by
> +must be suitably aligned: for most architectures a multiple of page
> +size is sufficient; however, some architectures may impose additional
> +restrictions. If the memory region specified by
>  .I addr
>  and
>  .I len
> @@ -226,6 +227,33 @@ Software that aspires to be portable should use this 
> option with care, keeping
>  in mind that the exact layout of a process' memory map is allowed to change
>  significantly between kernel versions, C library versions, and operating 
> system
>  releases.
> +.IP
> +Furthermore, this option is extremely hazardous (when used on its own), 
> because
> +it forcibly removes pre-existing mappings, making it easy for a 
> multi-threaded
> +process to corrupt its own address space.
> +.IP
> +For example, thread A looks through
> +.I /proc//maps
> +and locates an available
> +address range, while thread B simultaneously acquires part or all of that 
> same
> +address range. Thread A then calls mmap(MAP_FIXED), effectively overwriting
> +the mapping that thread B created.
> +.IP
> +Thread B need not create a mapping directly; simply making a library call
> +that, internally, uses
> +.I dlopen(3)
> +to load some other shared library, will
> +suffice. The dlopen(3) call will map the library into the process's address
> +space. Furthermore, almost any library call may be implemented using this
> +technique.
> +Examples include brk(2), malloc(3), pthread_create(3), and the PAM libraries
> +(http://www.linux-pam.org).
> +.IP
> +Newer kernels
> +(Linux 4.16 and later) have a
> +.B MAP_FIXED_SAFE
> +option that avoids the corruption problem; if available, MAP_FIXED_SAFE
> +should be preferred over MAP_FIXED.
>  .TP
>  .B MAP_GROWSDOWN
>  This flag is used for stacks.
> 


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

Re: [PATCH v5] mmap.2: MAP_FIXED updated documentation

2017-12-18 Thread Michael Kerrisk (man-pages) 
On 12/12/2017 01:23 AM, john.hubb...@gmail.com wrote:
> From: John Hubbard 
> 
> -- Expand the documentation to discuss the hazards in
>enough detail to allow avoiding them.
> 
> -- Mention the upcoming MAP_FIXED_SAFE flag.
> 
> -- Enhance the alignment requirement slightly.
> 
> CC: Michael Ellerman 
> CC: Jann Horn 
> CC: Matthew Wilcox 
> CC: Michal Hocko 
> CC: Mike Rapoport 
> CC: Cyril Hrubis 
> CC: Michal Hocko 
> CC: Pavel Machek 
> Signed-off-by: John Hubbard 

John,

Thanks for the patch. I think you win the prize for the 
most iterations ever on a man-pages patch! (And Michal,
thanks for helping out.) I've applied your patch, made 
some minor tweaks, and removed the mention of 
MAP_FIXED_SAFE, since I don't like to document stuff
that hasn't yet been merged. (I only later noticed the
fuss about the naming...)

Cheers,

Michael

> ---
> 
> Changes since v4:
> 
> -- v2 ("mmap.2: MAP_FIXED is no longer discouraged") was applied already,
>so v5 is a merge, including rewording of the paragraph transitions.
> 
> -- We seem to have consensus about what to say about alignment
>now, and this includes that new wording.
> 
> Changes since v3:
> 
> -- Removed the "how to use this safely" part, and
>the SHMLBA part, both as a result of Michal Hocko's
>review.
> 
> -- A few tiny wording fixes, at the not-quite-typo level.
> 
> Changes since v2:
> 
> -- Fixed up the "how to use safely" example, in response
>to Mike Rapoport's review.
> 
> -- Changed the alignment requirement from system page
>size, to SHMLBA. This was inspired by (but not yet
>recommended by) Cyril Hrubis' review.
> 
> -- Formatting: underlined /proc//maps
> 
> Changes since v1:
> 
> -- Covered topics recommended by Matthew Wilcox
>and Jann Horn, in their recent review: the hazards
>of overwriting pre-exising mappings, and some notes
>about how to use MAP_FIXED safely.
> 
> -- Rewrote the commit description accordingly.
> 
>  man2/mmap.2 | 32 ++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index a5a8eb47a..400cfda2d 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -212,8 +212,9 @@ Don't interpret
>  .I addr
>  as a hint: place the mapping at exactly that address.
>  .I addr
> -must be a multiple of the page size.
> -If the memory region specified by
> +must be suitably aligned: for most architectures a multiple of page
> +size is sufficient; however, some architectures may impose additional
> +restrictions. If the memory region specified by
>  .I addr
>  and
>  .I len
> @@ -226,6 +227,33 @@ Software that aspires to be portable should use this 
> option with care, keeping
>  in mind that the exact layout of a process' memory map is allowed to change
>  significantly between kernel versions, C library versions, and operating 
> system
>  releases.
> +.IP
> +Furthermore, this option is extremely hazardous (when used on its own), 
> because
> +it forcibly removes pre-existing mappings, making it easy for a 
> multi-threaded
> +process to corrupt its own address space.
> +.IP
> +For example, thread A looks through
> +.I /proc//maps
> +and locates an available
> +address range, while thread B simultaneously acquires part or all of that 
> same
> +address range. Thread A then calls mmap(MAP_FIXED), effectively overwriting
> +the mapping that thread B created.
> +.IP
> +Thread B need not create a mapping directly; simply making a library call
> +that, internally, uses
> +.I dlopen(3)
> +to load some other shared library, will
> +suffice. The dlopen(3) call will map the library into the process's address
> +space. Furthermore, almost any library call may be implemented using this
> +technique.
> +Examples include brk(2), malloc(3), pthread_create(3), and the PAM libraries
> +(http://www.linux-pam.org).
> +.IP
> +Newer kernels
> +(Linux 4.16 and later) have a
> +.B MAP_FIXED_SAFE
> +option that avoids the corruption problem; if available, MAP_FIXED_SAFE
> +should be preferred over MAP_FIXED.
>  .TP
>  .B MAP_GROWSDOWN
>  This flag is used for stacks.
> 


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

Re: [PATCH v5] mmap.2: MAP_FIXED updated documentation

2017-12-11 Thread Michal Hocko 
On Mon 11-12-17 16:23:31, john.hubb...@gmail.com wrote:
> From: John Hubbard 
> 
> -- Expand the documentation to discuss the hazards in
>enough detail to allow avoiding them.
> 
> -- Mention the upcoming MAP_FIXED_SAFE flag.
> 
> -- Enhance the alignment requirement slightly.
> 
> CC: Michael Ellerman 
> CC: Jann Horn 
> CC: Matthew Wilcox 
> CC: Michal Hocko 
> CC: Mike Rapoport 
> CC: Cyril Hrubis 
> CC: Michal Hocko 
> CC: Pavel Machek 
> Signed-off-by: John Hubbard 

Acked-by: Michal Hocko 

Thanks! I plan to submit my MAP_FIXED_FOO today and will send this
together with my mman update.

> ---
> 
> Changes since v4:
> 
> -- v2 ("mmap.2: MAP_FIXED is no longer discouraged") was applied already,
>so v5 is a merge, including rewording of the paragraph transitions.
> 
> -- We seem to have consensus about what to say about alignment
>now, and this includes that new wording.
> 
> Changes since v3:
> 
> -- Removed the "how to use this safely" part, and
>the SHMLBA part, both as a result of Michal Hocko's
>review.
> 
> -- A few tiny wording fixes, at the not-quite-typo level.
> 
> Changes since v2:
> 
> -- Fixed up the "how to use safely" example, in response
>to Mike Rapoport's review.
> 
> -- Changed the alignment requirement from system page
>size, to SHMLBA. This was inspired by (but not yet
>recommended by) Cyril Hrubis' review.
> 
> -- Formatting: underlined /proc//maps
> 
> Changes since v1:
> 
> -- Covered topics recommended by Matthew Wilcox
>and Jann Horn, in their recent review: the hazards
>of overwriting pre-exising mappings, and some notes
>about how to use MAP_FIXED safely.
> 
> -- Rewrote the commit description accordingly.
> 
>  man2/mmap.2 | 32 ++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index a5a8eb47a..400cfda2d 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -212,8 +212,9 @@ Don't interpret
>  .I addr
>  as a hint: place the mapping at exactly that address.
>  .I addr
> -must be a multiple of the page size.
> -If the memory region specified by
> +must be suitably aligned: for most architectures a multiple of page
> +size is sufficient; however, some architectures may impose additional
> +restrictions. If the memory region specified by
>  .I addr
>  and
>  .I len
> @@ -226,6 +227,33 @@ Software that aspires to be portable should use this 
> option with care, keeping
>  in mind that the exact layout of a process' memory map is allowed to change
>  significantly between kernel versions, C library versions, and operating 
> system
>  releases.
> +.IP
> +Furthermore, this option is extremely hazardous (when used on its own), 
> because
> +it forcibly removes pre-existing mappings, making it easy for a 
> multi-threaded
> +process to corrupt its own address space.
> +.IP
> +For example, thread A looks through
> +.I /proc//maps
> +and locates an available
> +address range, while thread B simultaneously acquires part or all of that 
> same
> +address range. Thread A then calls mmap(MAP_FIXED), effectively overwriting
> +the mapping that thread B created.
> +.IP
> +Thread B need not create a mapping directly; simply making a library call
> +that, internally, uses
> +.I dlopen(3)
> +to load some other shared library, will
> +suffice. The dlopen(3) call will map the library into the process's address
> +space. Furthermore, almost any library call may be implemented using this
> +technique.
> +Examples include brk(2), malloc(3), pthread_create(3), and the PAM libraries
> +(http://www.linux-pam.org).
> +.IP
> +Newer kernels
> +(Linux 4.16 and later) have a
> +.B MAP_FIXED_SAFE
> +option that avoids the corruption problem; if available, MAP_FIXED_SAFE
> +should be preferred over MAP_FIXED.
>  .TP
>  .B MAP_GROWSDOWN
>  This flag is used for stacks.
> -- 
> 2.15.1
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v5] mmap.2: MAP_FIXED updated documentation

2017-12-11 Thread Michal Hocko 
On Mon 11-12-17 16:23:31, john.hubb...@gmail.com wrote:
> From: John Hubbard 
> 
> -- Expand the documentation to discuss the hazards in
>enough detail to allow avoiding them.
> 
> -- Mention the upcoming MAP_FIXED_SAFE flag.
> 
> -- Enhance the alignment requirement slightly.
> 
> CC: Michael Ellerman 
> CC: Jann Horn 
> CC: Matthew Wilcox 
> CC: Michal Hocko 
> CC: Mike Rapoport 
> CC: Cyril Hrubis 
> CC: Michal Hocko 
> CC: Pavel Machek 
> Signed-off-by: John Hubbard 

Acked-by: Michal Hocko 

Thanks! I plan to submit my MAP_FIXED_FOO today and will send this
together with my mman update.

> ---
> 
> Changes since v4:
> 
> -- v2 ("mmap.2: MAP_FIXED is no longer discouraged") was applied already,
>so v5 is a merge, including rewording of the paragraph transitions.
> 
> -- We seem to have consensus about what to say about alignment
>now, and this includes that new wording.
> 
> Changes since v3:
> 
> -- Removed the "how to use this safely" part, and
>the SHMLBA part, both as a result of Michal Hocko's
>review.
> 
> -- A few tiny wording fixes, at the not-quite-typo level.
> 
> Changes since v2:
> 
> -- Fixed up the "how to use safely" example, in response
>to Mike Rapoport's review.
> 
> -- Changed the alignment requirement from system page
>size, to SHMLBA. This was inspired by (but not yet
>recommended by) Cyril Hrubis' review.
> 
> -- Formatting: underlined /proc//maps
> 
> Changes since v1:
> 
> -- Covered topics recommended by Matthew Wilcox
>and Jann Horn, in their recent review: the hazards
>of overwriting pre-exising mappings, and some notes
>about how to use MAP_FIXED safely.
> 
> -- Rewrote the commit description accordingly.
> 
>  man2/mmap.2 | 32 ++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index a5a8eb47a..400cfda2d 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -212,8 +212,9 @@ Don't interpret
>  .I addr
>  as a hint: place the mapping at exactly that address.
>  .I addr
> -must be a multiple of the page size.
> -If the memory region specified by
> +must be suitably aligned: for most architectures a multiple of page
> +size is sufficient; however, some architectures may impose additional
> +restrictions. If the memory region specified by
>  .I addr
>  and
>  .I len
> @@ -226,6 +227,33 @@ Software that aspires to be portable should use this 
> option with care, keeping
>  in mind that the exact layout of a process' memory map is allowed to change
>  significantly between kernel versions, C library versions, and operating 
> system
>  releases.
> +.IP
> +Furthermore, this option is extremely hazardous (when used on its own), 
> because
> +it forcibly removes pre-existing mappings, making it easy for a 
> multi-threaded
> +process to corrupt its own address space.
> +.IP
> +For example, thread A looks through
> +.I /proc//maps
> +and locates an available
> +address range, while thread B simultaneously acquires part or all of that 
> same
> +address range. Thread A then calls mmap(MAP_FIXED), effectively overwriting
> +the mapping that thread B created.
> +.IP
> +Thread B need not create a mapping directly; simply making a library call
> +that, internally, uses
> +.I dlopen(3)
> +to load some other shared library, will
> +suffice. The dlopen(3) call will map the library into the process's address
> +space. Furthermore, almost any library call may be implemented using this
> +technique.
> +Examples include brk(2), malloc(3), pthread_create(3), and the PAM libraries
> +(http://www.linux-pam.org).
> +.IP
> +Newer kernels
> +(Linux 4.16 and later) have a
> +.B MAP_FIXED_SAFE
> +option that avoids the corruption problem; if available, MAP_FIXED_SAFE
> +should be preferred over MAP_FIXED.
>  .TP
>  .B MAP_GROWSDOWN
>  This flag is used for stacks.
> -- 
> 2.15.1
> 

-- 
Michal Hocko
SUSE Labs

[PATCH v5] mmap.2: MAP_FIXED updated documentation

2017-12-11 Thread john . hubbard 
From: John Hubbard 

-- Expand the documentation to discuss the hazards in
   enough detail to allow avoiding them.

-- Mention the upcoming MAP_FIXED_SAFE flag.

-- Enhance the alignment requirement slightly.

CC: Michael Ellerman 
CC: Jann Horn 
CC: Matthew Wilcox 
CC: Michal Hocko 
CC: Mike Rapoport 
CC: Cyril Hrubis 
CC: Michal Hocko 
CC: Pavel Machek 
Signed-off-by: John Hubbard 
---

Changes since v4:

-- v2 ("mmap.2: MAP_FIXED is no longer discouraged") was applied already,
   so v5 is a merge, including rewording of the paragraph transitions.

-- We seem to have consensus about what to say about alignment
   now, and this includes that new wording.

Changes since v3:

-- Removed the "how to use this safely" part, and
   the SHMLBA part, both as a result of Michal Hocko's
   review.

-- A few tiny wording fixes, at the not-quite-typo level.

Changes since v2:

-- Fixed up the "how to use safely" example, in response
   to Mike Rapoport's review.

-- Changed the alignment requirement from system page
   size, to SHMLBA. This was inspired by (but not yet
   recommended by) Cyril Hrubis' review.

-- Formatting: underlined /proc//maps

Changes since v1:

-- Covered topics recommended by Matthew Wilcox
   and Jann Horn, in their recent review: the hazards
   of overwriting pre-exising mappings, and some notes
   about how to use MAP_FIXED safely.

-- Rewrote the commit description accordingly.

 man2/mmap.2 | 32 ++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/man2/mmap.2 b/man2/mmap.2
index a5a8eb47a..400cfda2d 100644
--- a/man2/mmap.2
+++ b/man2/mmap.2
@@ -212,8 +212,9 @@ Don't interpret
 .I addr
 as a hint: place the mapping at exactly that address.
 .I addr
-must be a multiple of the page size.
-If the memory region specified by
+must be suitably aligned: for most architectures a multiple of page
+size is sufficient; however, some architectures may impose additional
+restrictions. If the memory region specified by
 .I addr
 and
 .I len
@@ -226,6 +227,33 @@ Software that aspires to be portable should use this 
option with care, keeping
 in mind that the exact layout of a process' memory map is allowed to change
 significantly between kernel versions, C library versions, and operating system
 releases.
+.IP
+Furthermore, this option is extremely hazardous (when used on its own), because
+it forcibly removes pre-existing mappings, making it easy for a multi-threaded
+process to corrupt its own address space.
+.IP
+For example, thread A looks through
+.I /proc//maps
+and locates an available
+address range, while thread B simultaneously acquires part or all of that same
+address range. Thread A then calls mmap(MAP_FIXED), effectively overwriting
+the mapping that thread B created.
+.IP
+Thread B need not create a mapping directly; simply making a library call
+that, internally, uses
+.I dlopen(3)
+to load some other shared library, will
+suffice. The dlopen(3) call will map the library into the process's address
+space. Furthermore, almost any library call may be implemented using this
+technique.
+Examples include brk(2), malloc(3), pthread_create(3), and the PAM libraries
+(http://www.linux-pam.org).
+.IP
+Newer kernels
+(Linux 4.16 and later) have a
+.B MAP_FIXED_SAFE
+option that avoids the corruption problem; if available, MAP_FIXED_SAFE
+should be preferred over MAP_FIXED.
 .TP
 .B MAP_GROWSDOWN
 This flag is used for stacks.
-- 
2.15.1

[PATCH v5] mmap.2: MAP_FIXED updated documentation

2017-12-11 Thread john . hubbard 
From: John Hubbard 

-- Expand the documentation to discuss the hazards in
   enough detail to allow avoiding them.

-- Mention the upcoming MAP_FIXED_SAFE flag.

-- Enhance the alignment requirement slightly.

CC: Michael Ellerman 
CC: Jann Horn 
CC: Matthew Wilcox 
CC: Michal Hocko 
CC: Mike Rapoport 
CC: Cyril Hrubis 
CC: Michal Hocko 
CC: Pavel Machek 
Signed-off-by: John Hubbard 
---

Changes since v4:

-- v2 ("mmap.2: MAP_FIXED is no longer discouraged") was applied already,
   so v5 is a merge, including rewording of the paragraph transitions.

-- We seem to have consensus about what to say about alignment
   now, and this includes that new wording.

Changes since v3:

-- Removed the "how to use this safely" part, and
   the SHMLBA part, both as a result of Michal Hocko's
   review.

-- A few tiny wording fixes, at the not-quite-typo level.

Changes since v2:

-- Fixed up the "how to use safely" example, in response
   to Mike Rapoport's review.

-- Changed the alignment requirement from system page
   size, to SHMLBA. This was inspired by (but not yet
   recommended by) Cyril Hrubis' review.

-- Formatting: underlined /proc//maps

Changes since v1:

-- Covered topics recommended by Matthew Wilcox
   and Jann Horn, in their recent review: the hazards
   of overwriting pre-exising mappings, and some notes
   about how to use MAP_FIXED safely.

-- Rewrote the commit description accordingly.

 man2/mmap.2 | 32 ++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/man2/mmap.2 b/man2/mmap.2
index a5a8eb47a..400cfda2d 100644
--- a/man2/mmap.2
+++ b/man2/mmap.2
@@ -212,8 +212,9 @@ Don't interpret
 .I addr
 as a hint: place the mapping at exactly that address.
 .I addr
-must be a multiple of the page size.
-If the memory region specified by
+must be suitably aligned: for most architectures a multiple of page
+size is sufficient; however, some architectures may impose additional
+restrictions. If the memory region specified by
 .I addr
 and
 .I len
@@ -226,6 +227,33 @@ Software that aspires to be portable should use this 
option with care, keeping
 in mind that the exact layout of a process' memory map is allowed to change
 significantly between kernel versions, C library versions, and operating system
 releases.
+.IP
+Furthermore, this option is extremely hazardous (when used on its own), because
+it forcibly removes pre-existing mappings, making it easy for a multi-threaded
+process to corrupt its own address space.
+.IP
+For example, thread A looks through
+.I /proc//maps
+and locates an available
+address range, while thread B simultaneously acquires part or all of that same
+address range. Thread A then calls mmap(MAP_FIXED), effectively overwriting
+the mapping that thread B created.
+.IP
+Thread B need not create a mapping directly; simply making a library call
+that, internally, uses
+.I dlopen(3)
+to load some other shared library, will
+suffice. The dlopen(3) call will map the library into the process's address
+space. Furthermore, almost any library call may be implemented using this
+technique.
+Examples include brk(2), malloc(3), pthread_create(3), and the PAM libraries
+(http://www.linux-pam.org).
+.IP
+Newer kernels
+(Linux 4.16 and later) have a
+.B MAP_FIXED_SAFE
+option that avoids the corruption problem; if available, MAP_FIXED_SAFE
+should be preferred over MAP_FIXED.
 .TP
 .B MAP_GROWSDOWN
 This flag is used for stacks.
-- 
2.15.1