[PATCH 0/2] riscv: Increase mmap_rnd_bits_max on Sv48/57

[PATCH 0/2] riscv: Increase mmap_rnd_bits_max on Sv48/57

[Sami Tolvanen]

Hi all,

We noticed that 64-bit RISC-V kernels limit mmap_rnd_bits to 24
even if the hardware supports a larger virtual address space size
[1]. These two patches allow mmap_rnd_bits_max to be changed during
init, and bumps up the maximum randomness if we end up setting up
4/5-level paging at boot.

Sami

 [1] https://github.com/google/android-riscv64/issues/1


Sami Tolvanen (2):
  mm: Change mmap_rnd_bits_max to __ro_after_init
  riscv: mm: Update mmap_rnd_bits_max

 arch/riscv/mm/init.c | 6 ++++++
 include/linux/mm.h   | 2 +-
 mm/mmap.c            | 2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)


base-commit: 6465e260f48790807eef06b583b38ca9789b6072
-- 
2.42.0.582.g8ccd20d70d-goog

[PATCH 1/2] mm: Change mmap_rnd_bits_max to __ro_after_init

[Sami Tolvanen]

Allow mmap_rnd_bits_max to be updated on architectures that
determine virtual address space size at runtime instead of relying
on Kconfig options by changing it from const to __ro_after_init.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 include/linux/mm.h | 2 +-
 mm/mmap.c          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index bf5d0b1b16f4..72a98b2afaf9 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -86,7 +86,7 @@ extern int sysctl_legacy_va_layout;
 
 #ifdef CONFIG_HAVE_ARCH_MMAP_RND_BITS
 extern const int mmap_rnd_bits_min;
-extern const int mmap_rnd_bits_max;
+extern int mmap_rnd_bits_max __ro_after_init;
 extern int mmap_rnd_bits __read_mostly;
 #endif
 #ifdef CONFIG_HAVE_ARCH_MMAP_RND_COMPAT_BITS
diff --git a/mm/mmap.c b/mm/mmap.c
index b56a7f0c9f85..ed1b178b223a 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -64,7 +64,7 @@
 
 #ifdef CONFIG_HAVE_ARCH_MMAP_RND_BITS
 const int mmap_rnd_bits_min = CONFIG_ARCH_MMAP_RND_BITS_MIN;
-const int mmap_rnd_bits_max = CONFIG_ARCH_MMAP_RND_BITS_MAX;
+int mmap_rnd_bits_max __ro_after_init = CONFIG_ARCH_MMAP_RND_BITS_MAX;
 int mmap_rnd_bits __read_mostly = CONFIG_ARCH_MMAP_RND_BITS;
 #endif
 #ifdef CONFIG_HAVE_ARCH_MMAP_RND_COMPAT_BITS
-- 
2.42.0.582.g8ccd20d70d-goog

Re: [PATCH 1/2] mm: Change mmap_rnd_bits_max to __ro_after_init

[Kees Cook]

On Fri, Sep 29, 2023 at 09:11:57PM +0000, Sami Tolvanen wrote:
> Allow mmap_rnd_bits_max to be updated on architectures that
> determine virtual address space size at runtime instead of relying
> on Kconfig options by changing it from const to __ro_after_init.
> 
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>

Just the kind of thing __ro_after_init was made for. :)

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

-- 
Kees Cook

[PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

[Sami Tolvanen]

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(+)

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
 
 	/*
-- 
2.42.0.582.g8ccd20d70d-goog

Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

[Kees Cook]

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>

-- 
Kees Cook

Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

[Sami Tolvanen]

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

Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

[Kees Cook]

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


-- 
Kees Cook

Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

[Pedro Falcato]

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.

-- 
Pedro

Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

[Alexandre Ghiti]

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/

Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

[Sami Tolvanen]

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

Re: [PATCH 0/2] riscv: Increase mmap_rnd_bits_max on Sv48/57

[Palmer Dabbelt]

On Fri, 29 Sep 2023 14:11:56 PDT (-0700), samitolvanen@google.com wrote:
> Hi all,
>
> We noticed that 64-bit RISC-V kernels limit mmap_rnd_bits to 24
> even if the hardware supports a larger virtual address space size
> [1]. These two patches allow mmap_rnd_bits_max to be changed during
> init, and bumps up the maximum randomness if we end up setting up
> 4/5-level paging at boot.

Sorry for missing this, I'm just poking through old stuff in patchwork.  
As far as I can tell this is still relevant, the discussions are just on 
the mmap() bits (but we'd already screwed that one up and have since 
fixed it).

So

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>

in case someone else wants to take it, but I'm OK taking that MM patch 
with Kees' review.

>
> Sami
>
>  [1] https://github.com/google/android-riscv64/issues/1
>
>
> Sami Tolvanen (2):
>   mm: Change mmap_rnd_bits_max to __ro_after_init
>   riscv: mm: Update mmap_rnd_bits_max
>
>  arch/riscv/mm/init.c | 6 ++++++
>  include/linux/mm.h   | 2 +-
>  mm/mmap.c            | 2 +-
>  3 files changed, 8 insertions(+), 2 deletions(-)
>
>
> base-commit: 6465e260f48790807eef06b583b38ca9789b6072

Re: [PATCH 0/2] riscv: Increase mmap_rnd_bits_max on Sv48/57

[Kees Cook]

On Wed, Dec 06, 2023 at 05:14:26AM -0800, Palmer Dabbelt wrote:
> On Fri, 29 Sep 2023 14:11:56 PDT (-0700), samitolvanen@google.com wrote:
> > Hi all,
> > 
> > We noticed that 64-bit RISC-V kernels limit mmap_rnd_bits to 24
> > even if the hardware supports a larger virtual address space size
> > [1]. These two patches allow mmap_rnd_bits_max to be changed during
> > init, and bumps up the maximum randomness if we end up setting up
> > 4/5-level paging at boot.
> 
> Sorry for missing this, I'm just poking through old stuff in patchwork.  As
> far as I can tell this is still relevant, the discussions are just on the
> mmap() bits (but we'd already screwed that one up and have since fixed it).
> 
> So
> 
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
> 
> in case someone else wants to take it, but I'm OK taking that MM patch with
> Kees' review.

Yes, thanks! Please do. I already +1ed it:
https://lore.kernel.org/all/202309291454.436E19663@keescook

-Kees

-- 
Kees Cook

Re: [PATCH 0/2] riscv: Increase mmap_rnd_bits_max on Sv48/57

[Sami Tolvanen]

Hi Palmer,

On Wed, Dec 6, 2023 at 5:14 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Fri, 29 Sep 2023 14:11:56 PDT (-0700), samitolvanen@google.com wrote:
> > Hi all,
> >
> > We noticed that 64-bit RISC-V kernels limit mmap_rnd_bits to 24
> > even if the hardware supports a larger virtual address space size
> > [1]. These two patches allow mmap_rnd_bits_max to be changed during
> > init, and bumps up the maximum randomness if we end up setting up
> > 4/5-level paging at boot.
>
> Sorry for missing this, I'm just poking through old stuff in patchwork.
> As far as I can tell this is still relevant, the discussions are just on
> the mmap() bits (but we'd already screwed that one up and have since
> fixed it).
>
> So
>
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
>
> in case someone else wants to take it, but I'm OK taking that MM patch
> with Kees' review.

Is this still on your radar for v6.8 or would you prefer me to resend
the patches?

Sami

Re: [PATCH 0/2] riscv: Increase mmap_rnd_bits_max on Sv48/57

[patchwork-bot+linux-riscv]

Hello:

This series was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Fri, 29 Sep 2023 21:11:56 +0000 you wrote:
> Hi all,
> 
> We noticed that 64-bit RISC-V kernels limit mmap_rnd_bits to 24
> even if the hardware supports a larger virtual address space size
> [1]. These two patches allow mmap_rnd_bits_max to be changed during
> init, and bumps up the maximum randomness if we end up setting up
> 4/5-level paging at boot.
> 
> [...]

Here is the summary with links:
  - [1/2] mm: Change mmap_rnd_bits_max to __ro_after_init
    https://git.kernel.org/riscv/c/71a5849aedaa
  - [2/2] riscv: mm: Update mmap_rnd_bits_max
    https://git.kernel.org/riscv/c/7df1ff5a5cd6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html