linux-kernel - Re: [PATCH] mmap.2: document new MAP_FIXED

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 30 Nov 2017 09:23:14 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     John Hubbard <jhubbard@...dia.com>
Cc:     Michael Kerrisk <mtk.manpages@...il.com>,
        linux-api@...r.kernel.org, Khalid Aziz <khalid.aziz@...cle.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        Andrea Arcangeli <aarcange@...hat.com>, linux-mm@...ck.org,
        LKML <linux-kernel@...r.kernel.org>, linux-arch@...r.kernel.org,
        Florian Weimer <fweimer@...hat.com>
Subject: Re: [PATCH] mmap.2: document new MAP_FIXED_SAFE flag

On Wed 29-11-17 19:16:39, John Hubbard wrote:
[...]
> Hi Michal,
> 
> I've taken the liberty of mostly rewriting this part, in order to more closely 
> match the existing paragraphs; to fix minor typos; and to attempt to slightly
> clarify the paragraph.
> 
> +.BR MAP_FIXED_SAFE " (since Linux 4.16)"
> +Similar to MAP_FIXED with respect to the
> +.I
> +addr
> +enforcement, but different in that MAP_FIXED_SAFE never clobbers a pre-existing
> +mapped range. If the requested range would collide with an existing
> +mapping, then this call fails with
> +.B EEXIST.
> +This flag can therefore be used as a way to atomically (with respect to other
> +threads) attempt to map an address range: one thread will succeed; all others
> +will report failure. Please note that older kernels which do not recognize this
> +flag will typically (upon detecting a collision with a pre-existing mapping)
> +fall back a "non-MAP_FIXED" type of behavior: they will return an address that
> +is different than the requested one. Therefore, backward-compatible software
> +should check the returned address against the requested address.
> +.TP

I have taken yours. Thanks a lot!

> (I'm ignoring the naming, because there is another thread about that,
> so please just the above as "MAP_FIXED_whatever-is-chosen".)
> 
> > @@ -449,6 +461,12 @@ is not a valid file descriptor (and
> >  .B MAP_ANONYMOUS
> >  was not set).
> >  .TP
> > +.B EEXIST
> > +range covered by
> > +.IR addr , 
> 
> nit: trailing space on the above line.

fixed

> > +.IR length
> > +is clashing with an existing mapping.
> > +.TP
> >  .B EINVAL
> >  We don't like
> >  .IR addr ,
> > 
> 
> One other thing: reading through mmap.2, I now want to add this as well:
> 
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index 622a7000d..780cad6d9 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -222,20 +222,25 @@ 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.
> +Software that aspires to be as portable as possible should use this option with
> +care, keeping in mind that different kernels and C libraries may set up quite
> +different mapping ranges.
> 
> 
> ...because that advice is just wrong (it presumes that "less portable" ==
> "must be discouraged").
> 
> Should I send out a separate patch for that, or is it better to glom it together 
> with this one?

yes please
-- 
Michal Hocko
SUSE Labs