LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Alex Ghiti <alex@ghiti.fr>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>
Cc: linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-mm@kvack.org
Subject: Re: [PATCH 6/8] mm: Allow arch specific arch_randomize_brk() with CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
Date: Mon, 22 Nov 2021 12:47:13 +0100	[thread overview]
Message-ID: <2dd4a22e-f1a5-683c-2d17-ce726f03f4c5@csgroup.eu> (raw)
In-Reply-To: <325663a5-d9a1-a8b8-7f16-c2985c319864@ghiti.fr>



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.

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.

Thanks
Christophe

  reply	other threads:[~2021-11-22 11:47 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-22  8:48 [PATCH 0/8] Convert powerpc to default topdown mmap layout Christophe Leroy
2021-11-22  8:48 ` [PATCH 1/8] powerpc/mm: Make slice specific to book3s/64 Christophe Leroy
2021-11-22 14:48   ` kernel test robot
2021-11-24 12:10     ` Christophe Leroy
2021-11-24 13:49       ` Christophe Leroy
2021-11-26  5:15         ` [kbuild-all] " Chen, Rong A
2021-11-22 21:10   ` kernel test robot
2021-11-24 12:10     ` Christophe Leroy
2021-11-22  8:48 ` [PATCH 2/8] powerpc/mm: Remove CONFIG_PPC_MM_SLICES Christophe Leroy
2021-11-22  8:48 ` [PATCH 3/8] powerpc/mm: Remove asm/slice.h Christophe Leroy
2021-11-22  8:48 ` [PATCH 4/8] powerpc/mm: Move vma_mmu_pagesize() and hugetlb_get_unmapped_area() to slice.c Christophe Leroy
2021-11-22  8:48 ` [PATCH 5/8] powerpc/mm: Call radix__arch_get_unmapped_area() from arch_get_unmapped_area() Christophe Leroy
2021-11-22  8:48 ` [PATCH 6/8] mm: Allow arch specific arch_randomize_brk() with CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT Christophe Leroy
2021-11-22 11:22   ` Alex Ghiti
2021-11-22 11:47     ` Christophe Leroy [this message]
2021-11-22 12:57       ` Alexandre ghiti
2021-11-23  0:22   ` kernel test robot
2021-11-22  8:48 ` [PATCH 7/8] powerpc/mm: Convert to default topdown mmap layout Christophe Leroy
2021-11-22  8:48 ` [PATCH 8/8] powerpc/mm: Properly randomise mmap with slices Christophe Leroy
2021-11-24 13:21 ` [PATCH 0/8] Convert powerpc to default topdown mmap layout Nicholas Piggin
2021-11-24 13:40   ` Christophe Leroy
2021-11-24 18:00     ` Christophe Leroy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2dd4a22e-f1a5-683c-2d17-ce726f03f4c5@csgroup.eu \
    --to=christophe.leroy@csgroup.eu \
    --cc=alex@ghiti.fr \
    --cc=benh@kernel.crashing.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --subject='Re: [PATCH 6/8] mm: Allow arch specific arch_randomize_brk() with CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).