On Tue 05-12-17 19:14:34, john.hubbard@xxxxxxxxx wrote: > From: John Hubbard <jhubbard@xxxxxxxxxx> > > Previously, MAP_FIXED was "discouraged", due to portability > issues with the fixed address. In fact, there are other, more > serious issues. Also, alignment requirements were a bit vague. > So: > > -- 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. > > Some of the wording is lifted from Matthew Wilcox's review > (the "Portability issues" section). The alignment requirements > section uses Cyril Hrubis' wording, with light editing applied. > > Suggested-by: Matthew Wilcox <willy@xxxxxxxxxxxxx> > Suggested-by: Cyril Hrubis <chrubis@xxxxxxx> > Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx> Would you mind if I take this patch and resubmit it along with my MAP_FIXED_SAFE (or whatever name I will end up with) next week? Acked-by: Michal Hocko <mhocko@xxxxxxxx> > --- > > 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/<pid>/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 | 40 ++++++++++++++++++++++++++++++++++++---- > 1 file changed, 36 insertions(+), 4 deletions(-) > > diff --git a/man2/mmap.2 b/man2/mmap.2 > index 385f3bfd5..56b05cff1 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 > @@ -222,8 +223,39 @@ part of the existing mapping(s) will be discarded. > If the specified address cannot be used, > .BR mmap () > will fail. > -Because requiring a fixed address for a mapping is less portable, > -the use of this option is discouraged. > +.IP > +This option is extremely hazardous (when used on its own) and moderately > +non-portable. > +.IP > +Portability issues: a process's memory map may change significantly from one > +run to the next, depending on library versions, kernel versions and random > +numbers. > +.IP > +Hazards: this option 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/<pid>/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
