2023-09-29 21:22:38

by Sami Tolvanen

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

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


2023-09-29 23:14:27

by Sami Tolvanen

[permalink] [raw]
Subject: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

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 <[email protected]>
---
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

2023-09-29 23:17:38

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

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 <[email protected]>

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

Reviewed-by: Kees Cook <[email protected]>

--
Kees Cook

2023-09-30 09:02:53

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

On Fri, Sep 29, 2023 at 03:52:22PM -0700, Sami Tolvanen wrote:
> On Fri, Sep 29, 2023 at 2:54 PM Kees Cook <[email protected]> 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 <[email protected]>
> >
> > 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.


Attachments:
(No filename) (1.15 kB)
signature.asc (235.00 B)
Download all attachments

2023-09-30 14:45:35

by Sami Tolvanen

[permalink] [raw]
Subject: Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

On Fri, Sep 29, 2023 at 2:54 PM Kees Cook <[email protected]> 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 <[email protected]>
>
> 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

2023-10-01 04:32:04

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

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 <[email protected]> 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 <[email protected]>
> > >
> > > 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

2023-10-01 23:06:10

by Pedro Falcato

[permalink] [raw]
Subject: Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

On Sun, Oct 1, 2023 at 2:51 AM Kees Cook <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > >
> > > > 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

2023-10-02 07:04:23

by Alexandre Ghiti

[permalink] [raw]
Subject: Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

On 01/10/2023 17:19, Pedro Falcato wrote:
> On Sun, Oct 1, 2023 at 2:51 AM Kees Cook <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>>> 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/[email protected]/

2023-10-02 21:09:09

by Sami Tolvanen

[permalink] [raw]
Subject: Re: [PATCH 2/2] riscv: mm: Update mmap_rnd_bits_max

On Mon, Oct 2, 2023 at 12:02 AM Alexandre Ghiti <[email protected]> wrote:
>
> On 01/10/2023 17:19, Pedro Falcato wrote:
> > On Sun, Oct 1, 2023 at 2:51 AM Kees Cook <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >>>>> 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/[email protected]/

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

2023-10-02 22:46:14

by Sami Tolvanen

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

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 <[email protected]>
---
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

2023-12-06 13:14:34

by Palmer Dabbelt

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

On Fri, 29 Sep 2023 14:11:56 PDT (-0700), [email protected] 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 <[email protected]>
Acked-by: Palmer Dabbelt <[email protected]>

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

2023-12-06 20:29:12

by Kees Cook

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

On Wed, Dec 06, 2023 at 05:14:26AM -0800, Palmer Dabbelt wrote:
> On Fri, 29 Sep 2023 14:11:56 PDT (-0700), [email protected] 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 <[email protected]>
> Acked-by: Palmer Dabbelt <[email protected]>
>
> 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

2024-01-17 20:30:41

by Sami Tolvanen

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

Hi Palmer,

On Wed, Dec 6, 2023 at 5:14 AM Palmer Dabbelt <[email protected]> wrote:
>
> On Fri, 29 Sep 2023 14:11:56 PDT (-0700), [email protected] 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 <[email protected]>
> Acked-by: Palmer Dabbelt <[email protected]>
>
> 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

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

Hello:

This series was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <[email protected]>:

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