linux-kernel - Re: [PATCH 5/5] [RFC] mm: Remove MAP

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: <b7f7f849-00d1-49e5-8455-94eb9b45e273@redhat.com>
Date: Thu, 26 Sep 2024 10:46:50 +0200
From: David Hildenbrand <david@...hat.com>
To: Arnd Bergmann <arnd@...nel.org>, linux-mm@...ck.org
Cc: Arnd Bergmann <arnd@...db.de>, "Jason A. Donenfeld" <Jason@...c4.com>,
 Alexander Viro <viro@...iv.linux.org.uk>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Andreas Larsson <andreas@...sler.com>,
 Andrew Morton <akpm@...ux-foundation.org>, Ard Biesheuvel <ardb@...nel.org>,
 Christian Brauner <brauner@...nel.org>, Christoph Hellwig <hch@....de>,
 Christophe Leroy <christophe.leroy@...roup.eu>,
 Damien Le Moal <dlemoal@...nel.org>, Greg Ungerer <gerg@...ux-m68k.org>,
 Helge Deller <deller@....de>, Kees Cook <kees@...nel.org>,
 "Liam R . Howlett" <Liam.Howlett@...cle.com>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Matt Turner <mattst88@...il.com>, Max Filippov <jcmvbkbc@...il.com>,
 Michael Ellerman <mpe@...erman.id.au>, Michal Hocko <mhocko@...e.com>,
 Nicholas Piggin <npiggin@...il.com>,
 Richard Henderson <richard.henderson@...aro.org>,
 Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
 Vladimir Murzin <vladimir.murzin@....com>, Vlastimil Babka <vbabka@...e.cz>,
 linux-stm32@...md-mailman.stormreply.com, linux-kernel@...r.kernel.org,
 linux-mips@...r.kernel.org, linux-parisc@...r.kernel.org,
 linuxppc-dev@...ts.ozlabs.org, linux-arch@...r.kernel.org
Subject: Re: [PATCH 5/5] [RFC] mm: Remove MAP_UNINITIALIZED support

On 25.09.24 23:06, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
> 
> MAP_UNINITIALIZED was added back in 2009 for NOMMU kernels, specifically
> for blackfin, which is long gone. MAP_HUGE_SHIFT/MAP_HUGE_MASK were
> added in 2012 for architectures supporting hugepages, which at the time
> did not overlap with the ones supporting NOMMU.
> 
> Adding the macro under an #ifdef was obviously a mistake, which
> Christoph Hellwig tried to address by making it unconditionally defined
> to 0x4000000 as part of the series to support RISC-V NOMMU kernels. At
> this point linux/mman.h contained two conflicting definitions for bit 26,
> though the two are still mutually exclusive at runtime in all supported
> configurations.
> 
> According to the commit 854e9ed09ded ("mm: support madvise(MADV_FREE)")
> description, it was previously used internally by facebook, which
> would have resulted in MAP_HUGE_1MB turning into MAP_HUGE_2MB
> with MAP_UNINITIALIZED enabled, and every other page size implying
> MAP_UNINITIALIZED. I assume there are no remaining out of tree users
> on MMU-enabled kernels today.
> 
> I do not see any sensible way to redefine the macros for the ABI in
> a way avoids breaking something. The only ideas so far are:
> 
>   - do nothing, try to document the bug, hope for the best
> 
>   - remove the kernel implementation and redefine MAP_UNINITIALIZED to
>     zero in the header to silently turn it off for everyone. There are
>     few NOMMU users left, and the ones that do use NOMMU usually turn
>     off MMAP_ALLOW_UNINITIALIZED, as it still has the potential to cause
>     bugs and even security issues on systems with a memory protection
>     unit.
> 
>   - remove both the implementation and the macro to force a build
>     failure for anyone trying to use the feature. This way we can
>     see who complains and whether we need to put it back in some
>     form or change the userspace sources to no longer pass the flag.
> 

The first, uncontroversial step could indeed be to make 
MAP_UNINITIALIZED a nop, but still leave the definitions in mman.h etc 
around.

This is the same we did with MAP_DENYWRITE. There might be some weird 
user out there, and carelessly reusing the bit could result in trouble. 
(people might argue that they are not using it with MAP_HUGETLB, so it 
would work)

Going forward and removing MAP_UNINITIALIZED is a bit more 
controversial, but maybe there really isn't any other user around. 
Software that is not getting recompiled cannot be really identified by 
letting it rest in -next only.

My take would be to leave MAP_UNINITIALIZED in the headers in some form 
for documentation purposes.

-- 
Cheers,

David / dhildenb