linux-kernel - Re: [PATCH v2] mmap.2: MAP_FIXED updated documentation

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]

Message-ID: <2cff594a-b481-269d-dd91-ff2cc2f4100a@nvidia.com>
Date:   Mon, 4 Dec 2017 23:42:00 -0800
From:   John Hubbard <jhubbard@...dia.com>
To:     Michal Hocko <mhocko@...nel.org>
CC:     Cyril Hrubis <chrubis@...e.cz>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        linux-man <linux-man@...r.kernel.org>,
        <linux-api@...r.kernel.org>, Michael Ellerman <mpe@...erman.id.au>,
        <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>,
        <linux-arch@...r.kernel.org>, Jann Horn <jannh@...gle.com>,
        Matthew Wilcox <willy@...radead.org>
Subject: Re: [PATCH v2] mmap.2: MAP_FIXED updated documentation

On 12/04/2017 11:05 PM, Michal Hocko wrote:
> On Mon 04-12-17 18:14:18, John Hubbard wrote:
>> On 12/04/2017 02:55 AM, Cyril Hrubis wrote:
>>> Hi!
>>> I know that we are not touching the rest of the existing description for
>>> MAP_FIXED however the second sentence in the manual page says that "addr
>>> must be a multiple of the page size." Which however is misleading as
>>> this is not enough on some architectures. Code in the wild seems to
>>> (mis)use SHMLBA for aligment purposes but I'm not sure that we should
>>> advise something like that in the manpages.
>>>
>>> So what about something as:
>>>
>>> "addr must be suitably aligned, for most architectures multiple of page
>>> size is sufficient, however some may impose additional restrictions for
>>> page mapping addresses."
>>>
>>
>> Hi Cyril,
>>
>> Right, so I've been looking into this today, and I think we can go a bit
>> further than that, even. The kernel, as far back as the *original* git
>> commit in 2005, implements mmap on ARM by requiring that the address is
>> aligned to SHMLBA:
>>
>> arch/arm/mm/mmap.c:50:
>>
>> 	if (flags & MAP_FIXED) {
>> 		if (aliasing && flags & MAP_SHARED &&
>> 		    (addr - (pgoff << PAGE_SHIFT)) & (SHMLBA - 1))
>> 			return -EINVAL;
>> 		return addr;
>> 	}
>>
>> So, given that this has been the implementation for the last 12+ years (and
>> probably the whole time, in fact), I think we can be bold enough to use this
>> wording for the second sentence of MAP_FIXED:
>>
>> "addr must be a multiple of SHMLBA (<sys/shm.h>), which in turn is either
>> the system page size (on many architectures) or a multiple of the system
>> page size (on some architectures)."
>>
>> What do you think?
> 
> I am not sure this is a good idea. This is pulling way too many
> implementation details into the man page IMHO. Note that your wording is
> even incorrect because this applies only to shared mappings and on some
> architectures it even requires special memory regions. We do not want
> all that in the man page...
> 

Hi Michal,

OK, so it sounds like Cyril's original wording would be just about right,
after all, like this?

"addr must be suitably aligned. For most architectures multiple of page
size is sufficient; however, some may impose additional restrictions."

(It does seem unfortunate that the man page cannot help the programmer
actually write correct code here. He or she is forced to read the kernel
implementation, in order to figure out the true alignment rules. I was
hoping we could avoid that.)

thanks,
John Hubbard
NVIDIA