linux-hardening - Re: [PATCH] riscv: enable mseal sysmap for RV64

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <mhng-db41d38d-d3ec-4515-99f9-7368c82d46ca@palmer-ri-x1c9a>
Date: Thu, 08 May 2025 10:39:35 -0700 (PDT)
From: Palmer Dabbelt <palmer@...belt.com>
To: lorenzo.stoakes@...cle.com
CC: jeffxu@...omium.org, jszhang@...nel.org, akpm@...ux-foundation.org,
  Liam.Howlett@...cle.com, kees@...nel.org, Paul Walmsley <paul.walmsley@...ive.com>,
  aou@...s.berkeley.edu, alex@...ti.fr, linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
  linux-mm@...ck.org, linux-hardening@...r.kernel.org
Subject:     Re: [PATCH] riscv: enable mseal sysmap for RV64

On Wed, 07 May 2025 09:22:09 PDT (-0700), lorenzo.stoakes@...cle.com wrote:
> On Wed, May 07, 2025 at 09:18:31AM -0700, Jeff Xu wrote:
>> Hi Jisheng
>>
>> It seems mm maintainers might prefer arch change reviewed by arch
>> maintainer and goes to arch tree, according to discussion in [1], I
>> don't have an opinion on this,  adding mm maintainers as FYI.
>
> Thanks Jeff, appreciate the ping!
>
> Jisheng - the main point here is to ensure that the arch doesn't rely in any way
> on, within the arch code itself, relocating any of these mappings. It's pretty
> easy to eyeball it and get a sense.
>
> Because if this is the case, the arch will be broken by this change should a
> user enable it, and obviously we'd rather avoid that! :)
>
> It's really likely that you're fine, as it'd be unusual for an arch to do this,
> but I strongly suggest you do so.
>
> And yes, I think these should really go through arch trees as explicitly arch
> code.
>
> Thanks, Lorenzo
>
>>
>> On Sat, Apr 26, 2025 at 7:16 AM Jisheng Zhang <jszhang@...nel.org> wrote:
>> >
>> > Provide support for CONFIG_MSEAL_SYSTEM_MAPPINGS for RV64, covering the
>> > vdso, vvar.
>> >
>> > Passed sysmap_is_sealed and mseal_test self tests.
>> > Passed booting a buildroot rootfs image and a cli debian rootfs image.
>> >
>> mm maintainers like to get confirmation that the arch doesn't rely on
>> remapping the VDSO, VVAR, or any other special mappings, see
>> discussion in [2]

Do you have some description of what remapping is disallowed here?  
There's not a ton in that referenced thread.

We play a few tricks with remapping, including some aliasing to handle 
different VA widths and text patching (via poke pages).  IIRC those are 
similar in spirit to what's going on in x86/arm64 land, though sometimes 
the exact flavor of the trick matters.  If you've got something I can 
look at it might save me from having to read though the mm code...

and ya, we'll pick it up through the arch tree once one of us can be 
convinced this works ;)

>>
>> > Signed-off-by: Jisheng Zhang <jszhang@...nel.org>
>> > Cc: Jeff Xu <jeffxu@...omium.org>
>> > ---
>> >  arch/riscv/Kconfig       | 1 +
>> >  arch/riscv/kernel/vdso.c | 2 +-
>> >  2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> Please consider updating document as part of your patch:
>> features/core/mseal_sys_mappings/arch-support.txt
>> Documentation/userspace-api/mseal.rst
>>
>> Sample change in [3]
>>
>> >
>> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> > index bbec87b79309..3cb0b05eef62 100644
>> > --- a/arch/riscv/Kconfig
>> > +++ b/arch/riscv/Kconfig
>> > @@ -70,6 +70,7 @@ config RISCV
>> >         # LLD >= 14: https://github.com/llvm/llvm-project/issues/50505
>> >         select ARCH_SUPPORTS_LTO_CLANG if LLD_VERSION >= 140000
>> >         select ARCH_SUPPORTS_LTO_CLANG_THIN if LLD_VERSION >= 140000
>> > +       select ARCH_SUPPORTS_MSEAL_SYSTEM_MAPPINGS if 64BIT && MMU
>> The "if 64BIT && MMU" are not needed here.
>>
>> MMU is not checked by MSEAL_SYSTEM_MAPPINGS, which we should,  this
>> can go to security/Kconfig separately. If you'd like, please submit a
>> fix to mm tree directly.
>>
>> [1] https://lore.kernel.org/all/7EB087B72C4FBDD3+20250417132410.404043-1-wangyuli@uniontech.com/,
>> [2] https://lore.kernel.org/all/3de559d6-be19-44bc-ba8f-4c52d4bca684@lucifer.local/
>> [3] https://lore.kernel.org/all/648AB3031B5618C0+20250415153903.570662-1-wangyuli@uniontech.com/
>>
>> Thanks
>> -Jeff
>>
>> >         select ARCH_SUPPORTS_PAGE_TABLE_CHECK if MMU
>> >         select ARCH_SUPPORTS_PER_VMA_LOCK if MMU
>> >         select ARCH_SUPPORTS_RT
>> > diff --git a/arch/riscv/kernel/vdso.c b/arch/riscv/kernel/vdso.c
>> > index cc2895d1fbc2..3a8e038b10a2 100644
>> > --- a/arch/riscv/kernel/vdso.c
>> > +++ b/arch/riscv/kernel/vdso.c
>> > @@ -136,7 +136,7 @@ static int __setup_additional_pages(struct mm_struct *mm,
>> >
>> >         ret =
>> >            _install_special_mapping(mm, vdso_base, vdso_text_len,
>> > -               (VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC),
>> > +               (VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC | VM_SEALED_SYSMAP),
>> >                 vdso_info->cm);
>> >
>> >         if (IS_ERR(ret))
>> > --
>> > 2.47.2
>> >