diff mbox series

[2/2] riscv: mm: Update mmap_rnd_bits_max

Message ID	20230929211155.3910949-6-samitolvanen@google.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32; Date: Fri, 29 Sep 2023 21:11:58 +0000 In-Reply-To: <20230929211155.3910949-4-samitolvanen@google.com> Mime-Version: 1.0 References: <20230929211155.3910949-4-samitolvanen@google.com> Message-ID: <20230929211155.3910949-6-samitolvanen@google.com> Subject: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max From: Sami Tolvanen <samitolvanen@google.com> To: Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, Andrew Morton <akpm@linux-foundation.org>, Kees Cook <keescook@chromium.org> Cc: linux-mm@kvack.org, linux-riscv@lists.infradead.org, llvm@lists.linux.dev, linux-kernel@vger.kernel.org, Sami Tolvanen <samitolvanen@google.com> Content-Type: text/plain; charset="UTF-8" Precedence: bulk
Series	riscv: Increase mmap_rnd_bits_max on Sv48/57 \| [0/2] riscv: Increase mmap_rnd_bits_max on Sv48/57 [1/2] mm: Change mmap_rnd_bits_max to __ro_after_init [2/2] riscv: mm: Update mmap_rnd_bits_max

Commit Message

Sami Tolvanen Sept. 29, 2023, 9:11 p.m. UTC

  ARCH_MMAP_RND_BITS_MAX is based on Sv39, which leaves a few
potential bits of mmap randomness on the table if we end up enabling
4/5-level paging. Update mmap_rnd_bits_max to take the final address
space size into account. This increases mmap_rnd_bits_max from 24 to
33 with Sv48/57.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 arch/riscv/mm/init.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Kees Cook Sept. 29, 2023, 9:54 p.m. UTC | #1

On Fri, Sep 29, 2023 at 09:11:58PM +0000, Sami Tolvanen wrote:
> ARCH_MMAP_RND_BITS_MAX is based on Sv39, which leaves a few
> potential bits of mmap randomness on the table if we end up enabling
> 4/5-level paging. Update mmap_rnd_bits_max to take the final address
> space size into account. This increases mmap_rnd_bits_max from 24 to
> 33 with Sv48/57.
> 
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>

I like this. Is RISCV the only arch where the paging level can be chosen
at boot time?

Reviewed-by: Kees Cook <keescook@chromium.org>

Sami Tolvanen Sept. 29, 2023, 10:52 p.m. UTC | #2

On Fri, Sep 29, 2023 at 2:54 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Sep 29, 2023 at 09:11:58PM +0000, Sami Tolvanen wrote:
> > ARCH_MMAP_RND_BITS_MAX is based on Sv39, which leaves a few
> > potential bits of mmap randomness on the table if we end up enabling
> > 4/5-level paging. Update mmap_rnd_bits_max to take the final address
> > space size into account. This increases mmap_rnd_bits_max from 24 to
> > 33 with Sv48/57.
> >
> > Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
>
> I like this. Is RISCV the only arch where the paging level can be chosen
> at boot time?

I haven't seen this elsewhere, but I also haven't looked at all the
other architectures that closely. arm64 does something interesting
with ARM64_VA_BITS_52, but I think we can still handle that in
Kconfig.

Sami

Conor Dooley Sept. 30, 2023, 9:02 a.m. UTC | #3

On Fri, Sep 29, 2023 at 03:52:22PM -0700, Sami Tolvanen wrote:
> On Fri, Sep 29, 2023 at 2:54 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Fri, Sep 29, 2023 at 09:11:58PM +0000, Sami Tolvanen wrote:
> > > ARCH_MMAP_RND_BITS_MAX is based on Sv39, which leaves a few
> > > potential bits of mmap randomness on the table if we end up enabling
> > > 4/5-level paging. Update mmap_rnd_bits_max to take the final address
> > > space size into account. This increases mmap_rnd_bits_max from 24 to
> > > 33 with Sv48/57.
> > >
> > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> >
> > I like this. Is RISCV the only arch where the paging level can be chosen
> > at boot time?
> 
> I haven't seen this elsewhere, but I also haven't looked at all the
> other architectures that closely. arm64 does something interesting
> with ARM64_VA_BITS_52, but I think we can still handle that in
> Kconfig.

AFAIU, x86-64 can do this also:

	no4lvl		[RISCV] Disable 4-level and 5-level paging modes. Forces
			kernel to use 3-level paging instead.

	no5lvl		[X86-64,RISCV] Disable 5-level paging mode. Forces
			kernel to use 4-level paging instead.

Kees Cook Sept. 30, 2023, 9:01 p.m. UTC | #4

On Sat, Sep 30, 2023 at 10:02:35AM +0100, Conor Dooley wrote:
> On Fri, Sep 29, 2023 at 03:52:22PM -0700, Sami Tolvanen wrote:
> > On Fri, Sep 29, 2023 at 2:54 PM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > On Fri, Sep 29, 2023 at 09:11:58PM +0000, Sami Tolvanen wrote:
> > > > ARCH_MMAP_RND_BITS_MAX is based on Sv39, which leaves a few
> > > > potential bits of mmap randomness on the table if we end up enabling
> > > > 4/5-level paging. Update mmap_rnd_bits_max to take the final address
> > > > space size into account. This increases mmap_rnd_bits_max from 24 to
> > > > 33 with Sv48/57.
> > > >
> > > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> > >
> > > I like this. Is RISCV the only arch where the paging level can be chosen
> > > at boot time?
> > 
> > I haven't seen this elsewhere, but I also haven't looked at all the
> > other architectures that closely. arm64 does something interesting
> > with ARM64_VA_BITS_52, but I think we can still handle that in
> > Kconfig.
> 
> AFAIU, x86-64 can do this also:
> 
> 	no4lvl		[RISCV] Disable 4-level and 5-level paging modes. Forces
> 			kernel to use 3-level paging instead.
> 
> 	no5lvl		[X86-64,RISCV] Disable 5-level paging mode. Forces
> 			kernel to use 4-level paging instead.

Ah-ha! Okay, well, then let's track this idea:
https://github.com/KSPP/linux/issues/346

Pedro Falcato Oct. 1, 2023, 3:19 p.m. UTC | #5

On Sun, Oct 1, 2023 at 2:51 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Sat, Sep 30, 2023 at 10:02:35AM +0100, Conor Dooley wrote:
> > On Fri, Sep 29, 2023 at 03:52:22PM -0700, Sami Tolvanen wrote:
> > > On Fri, Sep 29, 2023 at 2:54 PM Kees Cook <keescook@chromium.org> wrote:
> > > >
> > > > On Fri, Sep 29, 2023 at 09:11:58PM +0000, Sami Tolvanen wrote:
> > > > > ARCH_MMAP_RND_BITS_MAX is based on Sv39, which leaves a few
> > > > > potential bits of mmap randomness on the table if we end up enabling
> > > > > 4/5-level paging. Update mmap_rnd_bits_max to take the final address
> > > > > space size into account. This increases mmap_rnd_bits_max from 24 to
> > > > > 33 with Sv48/57.
> > > > >
> > > > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> > > >
> > > > I like this. Is RISCV the only arch where the paging level can be chosen
> > > > at boot time?
> > >
> > > I haven't seen this elsewhere, but I also haven't looked at all the
> > > other architectures that closely. arm64 does something interesting
> > > with ARM64_VA_BITS_52, but I think we can still handle that in
> > > Kconfig.
> >
> > AFAIU, x86-64 can do this also:
> >
> >       no4lvl          [RISCV] Disable 4-level and 5-level paging modes. Forces
> >                       kernel to use 3-level paging instead.
> >
> >       no5lvl          [X86-64,RISCV] Disable 5-level paging mode. Forces
> >                       kernel to use 4-level paging instead.
>
> Ah-ha! Okay, well, then let's track this idea:
> https://github.com/KSPP/linux/issues/346

(Replying here for visibility, tell me if you want to move this
discussion to github)

AIUI, x86 cannot do this for compat reasons. Even if you enable LA57,
mmap only gives you < 48-bit addresses, for compatibility with things
like JITs, etc that stash information in the upper 16 bits. You need
to pass a > 48-bit mmap hint to get 57-bit addresses.

I imagine riscv does not have this issue yet, due to little
accumulated cruft, but it may be wise to check against popular JITters
for these problems on riscv code.

Alexandre Ghiti Oct. 2, 2023, 7:02 a.m. UTC | #6

On 01/10/2023 17:19, Pedro Falcato wrote:
> On Sun, Oct 1, 2023 at 2:51 AM Kees Cook <keescook@chromium.org> wrote:
>> On Sat, Sep 30, 2023 at 10:02:35AM +0100, Conor Dooley wrote:
>>> On Fri, Sep 29, 2023 at 03:52:22PM -0700, Sami Tolvanen wrote:
>>>> On Fri, Sep 29, 2023 at 2:54 PM Kees Cook <keescook@chromium.org> wrote:
>>>>> On Fri, Sep 29, 2023 at 09:11:58PM +0000, Sami Tolvanen wrote:
>>>>>> ARCH_MMAP_RND_BITS_MAX is based on Sv39, which leaves a few
>>>>>> potential bits of mmap randomness on the table if we end up enabling
>>>>>> 4/5-level paging. Update mmap_rnd_bits_max to take the final address
>>>>>> space size into account. This increases mmap_rnd_bits_max from 24 to
>>>>>> 33 with Sv48/57.
>>>>>>
>>>>>> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
>>>>> I like this. Is RISCV the only arch where the paging level can be chosen
>>>>> at boot time?
>>>> I haven't seen this elsewhere, but I also haven't looked at all the
>>>> other architectures that closely. arm64 does something interesting
>>>> with ARM64_VA_BITS_52, but I think we can still handle that in
>>>> Kconfig.
>>> AFAIU, x86-64 can do this also:
>>>
>>>        no4lvl          [RISCV] Disable 4-level and 5-level paging modes. Forces
>>>                        kernel to use 3-level paging instead.
>>>
>>>        no5lvl          [X86-64,RISCV] Disable 5-level paging mode. Forces
>>>                        kernel to use 4-level paging instead.
>> Ah-ha! Okay, well, then let's track this idea:
>> https://github.com/KSPP/linux/issues/346
> (Replying here for visibility, tell me if you want to move this
> discussion to github)
>
> AIUI, x86 cannot do this for compat reasons. Even if you enable LA57,
> mmap only gives you < 48-bit addresses, for compatibility with things
> like JITs, etc that stash information in the upper 16 bits. You need
> to pass a > 48-bit mmap hint to get 57-bit addresses.
>
> I imagine riscv does not have this issue yet, due to little
> accumulated cruft, but it may be wise to check against popular JITters
> for these problems on riscv code.
>

We already encountered those issues and the same solution was recently 
merged (restrict to sv48 unless otherwise specified): 
https://lore.kernel.org/all/20230809232218.849726-1-charlie@rivosinc.com/

Sami Tolvanen Oct. 2, 2023, 3:58 p.m. UTC | #7

On Mon, Oct 2, 2023 at 12:02 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> On 01/10/2023 17:19, Pedro Falcato wrote:
> > On Sun, Oct 1, 2023 at 2:51 AM Kees Cook <keescook@chromium.org> wrote:
> >> On Sat, Sep 30, 2023 at 10:02:35AM +0100, Conor Dooley wrote:
> >>> On Fri, Sep 29, 2023 at 03:52:22PM -0700, Sami Tolvanen wrote:
> >>>> On Fri, Sep 29, 2023 at 2:54 PM Kees Cook <keescook@chromium.org> wrote:
> >>>>> On Fri, Sep 29, 2023 at 09:11:58PM +0000, Sami Tolvanen wrote:
> >>>>>> ARCH_MMAP_RND_BITS_MAX is based on Sv39, which leaves a few
> >>>>>> potential bits of mmap randomness on the table if we end up enabling
> >>>>>> 4/5-level paging. Update mmap_rnd_bits_max to take the final address
> >>>>>> space size into account. This increases mmap_rnd_bits_max from 24 to
> >>>>>> 33 with Sv48/57.
> >>>>>>
> >>>>>> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> >>>>> I like this. Is RISCV the only arch where the paging level can be chosen
> >>>>> at boot time?
> >>>> I haven't seen this elsewhere, but I also haven't looked at all the
> >>>> other architectures that closely. arm64 does something interesting
> >>>> with ARM64_VA_BITS_52, but I think we can still handle that in
> >>>> Kconfig.
> >>> AFAIU, x86-64 can do this also:
> >>>
> >>>        no4lvl          [RISCV] Disable 4-level and 5-level paging modes. Forces
> >>>                        kernel to use 3-level paging instead.
> >>>
> >>>        no5lvl          [X86-64,RISCV] Disable 5-level paging mode. Forces
> >>>                        kernel to use 4-level paging instead.
> >> Ah-ha! Okay, well, then let's track this idea:
> >> https://github.com/KSPP/linux/issues/346
> > (Replying here for visibility, tell me if you want to move this
> > discussion to github)
> >
> > AIUI, x86 cannot do this for compat reasons. Even if you enable LA57,
> > mmap only gives you < 48-bit addresses, for compatibility with things
> > like JITs, etc that stash information in the upper 16 bits. You need
> > to pass a > 48-bit mmap hint to get 57-bit addresses.
> >
> > I imagine riscv does not have this issue yet, due to little
> > accumulated cruft, but it may be wise to check against popular JITters
> > for these problems on riscv code.
> >
>
> We already encountered those issues and the same solution was recently
> merged (restrict to sv48 unless otherwise specified):
> https://lore.kernel.org/all/20230809232218.849726-1-charlie@rivosinc.com/

We recently ran into this issue when bringing up Android as well
because qemu defaults to Sv57 and some userspace bits weren't happy
with >48-bit mmap addresses.

Note that this patch uses MMAP_VA_BITS, which is 48 for both Sv48 and
Sv57, which is why mmap_rnd_bits_max will be 33 even with Sv57.

Sami

diff mbox series

Patch

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 0798bd861dcb..ff8d21a6cb2d 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -762,6 +762,11 @@  static int __init print_no5lvl(char *p)
 }
 early_param("no5lvl", print_no5lvl);
 
+static void __init set_mmap_rnd_bits_max(void)
+{
+	mmap_rnd_bits_max = MMAP_VA_BITS - PAGE_SHIFT - 3;
+}
+
 /*
  * There is a simple way to determine if 4-level is supported by the
  * underlying hardware: establish 1:1 mapping in 4-level page table mode
@@ -1071,6 +1076,7 @@  asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 
 #if defined(CONFIG_64BIT) && !defined(CONFIG_XIP_KERNEL)
 	set_satp_mode(dtb_pa);
+	set_mmap_rnd_bits_max();
 #endif
 
 	/*