linux-kernel - Re: [PATCH 6/8] mm: Allow arch specific arch_randomize_brk() with CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP

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: <eb070ba0-2131-cc4e-4742-ec20c13d72ef@ghiti.fr>
Date:   Mon, 22 Nov 2021 13:57:56 +0100
From:   Alexandre ghiti <alex@...ti.fr>
To:     Christophe Leroy <christophe.leroy@...roup.eu>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>
Cc:     linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-mm@...ck.org
Subject: Re: [PATCH 6/8] mm: Allow arch specific arch_randomize_brk() with
 CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT

On 11/22/21 12:47, Christophe Leroy wrote:
>
>
> Le 22/11/2021 à 12:22, Alex Ghiti a écrit :
>> Hi Christophe,
>>
>> Le 22/11/2021 à 09:48, Christophe Leroy a écrit :
>>> Commit e7142bf5d231 ("arm64, mm: make randomization selected by
>>> generic topdown mmap layout") introduced a default version of
>>> arch_randomize_brk() provided when
>>> CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT is selected.
>>>
>>> powerpc could select CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
>>> but needs to provide its own arch_randomize_brk().
>>>
>>> In order to allow that, don't make
>>> CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT select
>>> CONFIG_ARCH_HAS_ELF_RANDOMIZE. Instead, ensure that
>>> selecting CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT and
>>> selecting CONFIG_ARCH_HAS_ELF_RANDOMIZE has the same effect.
>>
>> This feels weird to me since if CONFIG_ARCH_HAS_ELF_RANDOMIZE is used 
>> somewhere else at some point, it is not natural to add 
>> CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT: can't we use a __weak 
>> function or a new CONFIG_ARCH_HAS_RANDOMIZE_BRK?
>
>
> Yes I also found things a bit weird.
>
> CONFIG_ARCH_HAS_RANDOMIZE_BRK could be an idea but how different would 
> it be from CONFIG_ARCH_HAS_ELF_RANDOMIZE ? In fact I find it weird 
> that CONFIG_ARCH_HAS_ELF_RANDOMIZE is selected by 
> CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT and not by the arch itself.


IIRC, this was a request from Kees Cook who wanted to enforce this 
security measure.


>
> On the other hand CONFIG_ARCH_HAS_ELF_RANDOMIZE also handles 
> arch_mmap_rnd() and here we are talking about arch_randomize_brk() only.
>
> In the begining I was thinking about adding a 
> CONFIG_ARCH_WANT_DEFAULT_RANDOMIZE_BRK, but it was meaning adding it 
> to the few other arches selecting 
> CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT.
>
> So I think I will go for the __weak function option.


Ok, thanks.

Alex


>
> Thanks
> Christophe