On Wed, Apr 16, 2025 at 4:03 PM Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> wrote: > > On Wed, Apr 16, 2025 at 03:53:51PM +0800, Huacai Chen wrote: > > Hi, Lorenzo, > > > > On Tue, Apr 15, 2025 at 11:53 PM Lorenzo Stoakes > > <lorenzo.stoakes@xxxxxxxxxx> wrote: > > > > > > On Tue, Apr 15, 2025 at 11:39:03PM +0800, WangYuli wrote: > > > > Provide support for CONFIG_MSEAL_SYSTEM_MAPPINGS on LoongArch, > > > > covering the vdso. > > > > > > I've also checked and determined that, as far as I can tell, the loongarch > > > arch-specific doe don't appear at any point to rely upon remapping the VDSO > > > or VVAR areas so sealing these should not be problematic. > > What does "remapping the VDSO" mean here? There is a function > > vdso_mremap() in arch/loongarch/kernel/vdso.c. > > It means remapping the VDSO (and VVAR) since VMA sealing prevents this. VMA > sealing means that you map, and that's it until process tear down, more or > less... > > I mean this is the thing, in my view, anybody enabling a feature that > prevents you from doing X with Y should understand that this is the case, > and explicitly state that no - it appears that we don't need to do X with > Y - in any legitimate operation. > > It seems that so far, I am yet to encounter anybody enabling this feature > who does. Which is concerning. But by the by. I guess I will continue to > have to say the same thing to everybody and then go check it myself each > time :) > > Anyway, what you're referring to is a hook that is invoked when _userland_ > tries to remap the VDSO, which will now be prevented, if the user enables > this feature. > > So those using this feature will break a bunch of userspace, namely anybody > who wants/needs to remap the VDSO/VVAR etc. examples are CRIU, rr, etc. > > So this is fine. > > The problem would be if the _arch_ code itself needed to remap or move the > VDSO or VVAR around. This would be odd, and I think we know of only one > case (and even then it's uncertain), but it's important that people > explicitly check this. > > _As far as I can tell_, loongarch doesn't need to do this so it is safe to > enable this there, given the feature is opt-in. > > But this kind of concern is precisely why we need arch maintainers to check > this. > > I did insist on these limitations and concerns being placed in the > documentation and Kconfig entries so people are aware on review. OK, I know. In V1 you suggested this patch to go through the arch tree. But I saw Andrew has already taken it, what should I do now? Huacai > > Thanks, Lorenzo > > > > > Huacai > > > > > > > > > > > > > Link: https://lore.kernel.org/all/25bad37f-273e-4626-999c-e1890be96182@lucifer.local/ > > > > Tested-by: Yuli Wang <wangyuli@xxxxxxxxxxxxx> > > > > Signed-off-by: Yuli Wang <wangyuli@xxxxxxxxxxxxx> > > > > > > LGTM, > > > > > > Acked-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> > > > > > > But let's get some R-b's from the arch people please! > > > > > > > --- > > > > Changelog: > > > > *v1->v2: Modify mseal_sys_mappings/arch-support.txt. > > > > --- > > > > .../features/core/mseal_sys_mappings/arch-support.txt | 2 +- > > > > Documentation/userspace-api/mseal.rst | 2 +- > > > > arch/loongarch/Kconfig | 1 + > > > > arch/loongarch/kernel/vdso.c | 4 +++- > > > > 4 files changed, 6 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/Documentation/features/core/mseal_sys_mappings/arch-support.txt b/Documentation/features/core/mseal_sys_mappings/arch-support.txt > > > > index c6cab9760d57..a3c24233eb9b 100644 > > > > --- a/Documentation/features/core/mseal_sys_mappings/arch-support.txt > > > > +++ b/Documentation/features/core/mseal_sys_mappings/arch-support.txt > > > > @@ -12,7 +12,7 @@ > > > > | arm64: | ok | > > > > | csky: | N/A | > > > > | hexagon: | N/A | > > > > - | loongarch: | TODO | > > > > + | loongarch: | ok | > > > > | m68k: | N/A | > > > > | microblaze: | N/A | > > > > | mips: | TODO | > > > > diff --git a/Documentation/userspace-api/mseal.rst b/Documentation/userspace-api/mseal.rst > > > > index 1dabfc29be0d..ef733f69003d 100644 > > > > --- a/Documentation/userspace-api/mseal.rst > > > > +++ b/Documentation/userspace-api/mseal.rst > > > > @@ -144,7 +144,7 @@ Use cases > > > > architecture. > > > > > > > > The following architectures currently support this feature: x86-64, arm64, > > > > - and s390. > > > > + loongarch and s390. > > > > > > > > WARNING: This feature breaks programs which rely on relocating > > > > or unmapping system mappings. Known broken software at the time > > > > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig > > > > index 067c0b994648..54ed5b59a690 100644 > > > > --- a/arch/loongarch/Kconfig > > > > +++ b/arch/loongarch/Kconfig > > > > @@ -69,6 +69,7 @@ config LOONGARCH > > > > select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > > > > select ARCH_SUPPORTS_LTO_CLANG > > > > select ARCH_SUPPORTS_LTO_CLANG_THIN > > > > + select ARCH_SUPPORTS_MSEAL_SYSTEM_MAPPINGS > > > > select ARCH_SUPPORTS_NUMA_BALANCING > > > > select ARCH_SUPPORTS_RT > > > > select ARCH_USE_BUILTIN_BSWAP > > > > diff --git a/arch/loongarch/kernel/vdso.c b/arch/loongarch/kernel/vdso.c > > > > index 10cf1608c7b3..7b888d9085a0 100644 > > > > --- a/arch/loongarch/kernel/vdso.c > > > > +++ b/arch/loongarch/kernel/vdso.c > > > > @@ -105,7 +105,9 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp) > > > > > > > > vdso_addr = data_addr + VVAR_SIZE; > > > > vma = _install_special_mapping(mm, vdso_addr, info->size, > > > > - VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC, > > > > + VM_READ | VM_EXEC | > > > > + VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC | > > > > + VM_SEALED_SYSMAP, > > > > &info->code_mapping); > > > > if (IS_ERR(vma)) { > > > > ret = PTR_ERR(vma); > > > > -- > > > > 2.49.0 > > > >
