Messages in this thread | | | Date | Mon, 22 Nov 2021 13:57:56 +0100 | Subject | Re: [PATCH 6/8] mm: Allow arch specific arch_randomize_brk() with CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT | From | Alexandre ghiti <> |
| |
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
| |