[PATCH mm-unstable 0/2] mseal system mappings fix + s390 enablement

[PATCH mm-unstable 0/2] mseal system mappings fix + s390 enablement

[Heiko Carstens]

When rebasing the mseal series on top of the generic vdso data storage
the VM_SEALED_SYSMAP vm flag for the vvar mapping got lost. Add that again.

Also add s390 support for MSEAL_SYSTEM_MAPPINGS.

Heiko Carstens (2):
  mseal sysmap: generic vdso vvar mapping
  mseal sysmap: enable s390

 arch/s390/Kconfig       | 1 +
 arch/s390/kernel/vdso.c | 2 +-
 lib/vdso/datastore.c    | 3 ++-
 3 files changed, 4 insertions(+), 2 deletions(-)

-- 
2.45.2

[PATCH mm-unstable 1/2] mseal sysmap: generic vdso vvar mapping

[Heiko Carstens]

With the introduction of the generic vdso data storage the VM_SEALED_SYSMAP
vm flag must be moved from the architecture specific
_install_special_mapping() call [1] [2] which maps the vvar mapping to
generic code.

[1] https://lkml.kernel.org/r/20250305021711.3867874-4-jeffxu@google.com
[2] https://lkml.kernel.org/r/20250305021711.3867874-5-jeffxu@google.com

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 lib/vdso/datastore.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/vdso/datastore.c b/lib/vdso/datastore.c
index e227fbbcb796..b7c7386c98a9 100644
--- a/lib/vdso/datastore.c
+++ b/lib/vdso/datastore.c
@@ -99,7 +99,8 @@ const struct vm_special_mapping vdso_vvar_mapping = {
 struct vm_area_struct *vdso_install_vvar_mapping(struct mm_struct *mm, unsigned long addr)
 {
 	return _install_special_mapping(mm, addr, VDSO_NR_PAGES * PAGE_SIZE,
-					VM_READ | VM_MAYREAD | VM_IO | VM_DONTDUMP | VM_PFNMAP,
+					VM_READ | VM_MAYREAD | VM_IO | VM_DONTDUMP |
+					VM_PFNMAP | VM_SEALED_SYSMAP,
 					&vdso_vvar_mapping);
 }
 
-- 
2.45.2

Re: [PATCH mm-unstable 1/2] mseal sysmap: generic vdso vvar mapping

[Thomas Weißschuh]

Hi Heiko,

On Tue, Mar 11, 2025 at 01:33:25PM +0100, Heiko Carstens wrote:
> With the introduction of the generic vdso data storage the VM_SEALED_SYSMAP
> vm flag must be moved from the architecture specific
> _install_special_mapping() call [1] [2] which maps the vvar mapping to
> generic code.

I think this change should be part of the merge commit between the
trees carrying the generic vdso data storage and mseal for system mappings.

https://lore.kernel.org/lkml/20250307151426.5f3c0c39@canb.auug.org.au/
https://lore.kernel.org/lkml/20250311150847.5a63db36@canb.auug.org.au/


> [1] https://lkml.kernel.org/r/20250305021711.3867874-4-jeffxu@google.com
> [2] https://lkml.kernel.org/r/20250305021711.3867874-5-jeffxu@google.com
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  lib/vdso/datastore.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/vdso/datastore.c b/lib/vdso/datastore.c
> index e227fbbcb796..b7c7386c98a9 100644
> --- a/lib/vdso/datastore.c
> +++ b/lib/vdso/datastore.c
> @@ -99,7 +99,8 @@ const struct vm_special_mapping vdso_vvar_mapping = {
>  struct vm_area_struct *vdso_install_vvar_mapping(struct mm_struct *mm, unsigned long addr)
>  {
>  	return _install_special_mapping(mm, addr, VDSO_NR_PAGES * PAGE_SIZE,
> -					VM_READ | VM_MAYREAD | VM_IO | VM_DONTDUMP | VM_PFNMAP,
> +					VM_READ | VM_MAYREAD | VM_IO | VM_DONTDUMP |
> +					VM_PFNMAP | VM_SEALED_SYSMAP,
>  					&vdso_vvar_mapping);
>  }
>  
> -- 
> 2.45.2
> 

Re: [PATCH mm-unstable 1/2] mseal sysmap: generic vdso vvar mapping

[Heiko Carstens]

On Tue, Mar 11, 2025 at 01:59:21PM +0100, Thomas Weißschuh wrote:
> Hi Heiko,
> 
> On Tue, Mar 11, 2025 at 01:33:25PM +0100, Heiko Carstens wrote:
> > With the introduction of the generic vdso data storage the VM_SEALED_SYSMAP
> > vm flag must be moved from the architecture specific
> > _install_special_mapping() call [1] [2] which maps the vvar mapping to
> > generic code.
> 
> I think this change should be part of the merge commit between the
> trees carrying the generic vdso data storage and mseal for system mappings.
> 
> https://lore.kernel.org/lkml/20250307151426.5f3c0c39@canb.auug.org.au/
> https://lore.kernel.org/lkml/20250311150847.5a63db36@canb.auug.org.au/

Yes, but Andrew added your patches into his tree and on top of that
the mseal patches:
https://lore.kernel.org/all/20250306235802.ff0f406acd0117bcfe927082@linux-foundation.org/
https://web.git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/log/?h=mm-unstable

When doing that the vvar bits got lost. I don't know what Andrew's
plans for the merge window are, but right now there is no merge commit
which could be fixed.

Re: [PATCH mm-unstable 1/2] mseal sysmap: generic vdso vvar mapping

[Lorenzo Stoakes]

On Tue, Mar 11, 2025 at 01:33:25PM +0100, Heiko Carstens wrote:
> With the introduction of the generic vdso data storage the VM_SEALED_SYSMAP
> vm flag must be moved from the architecture specific
> _install_special_mapping() call [1] [2] which maps the vvar mapping to
> generic code.
>
> [1] https://lkml.kernel.org/r/20250305021711.3867874-4-jeffxu@google.com
> [2] https://lkml.kernel.org/r/20250305021711.3867874-5-jeffxu@google.com
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Since CONFIG_MSEAL_SYSTEM_MAPPINGS this relies upon
ARCH_SUPPORTS_MSEAL_SYSTEM_MAPPINGS, this is safe to do in generic code
(otherwise VM_SEALED_SYSMAP is VM_NONE).

So all good and:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  lib/vdso/datastore.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/vdso/datastore.c b/lib/vdso/datastore.c
> index e227fbbcb796..b7c7386c98a9 100644
> --- a/lib/vdso/datastore.c
> +++ b/lib/vdso/datastore.c
> @@ -99,7 +99,8 @@ const struct vm_special_mapping vdso_vvar_mapping = {
>  struct vm_area_struct *vdso_install_vvar_mapping(struct mm_struct *mm, unsigned long addr)
>  {
>  	return _install_special_mapping(mm, addr, VDSO_NR_PAGES * PAGE_SIZE,
> -					VM_READ | VM_MAYREAD | VM_IO | VM_DONTDUMP | VM_PFNMAP,
> +					VM_READ | VM_MAYREAD | VM_IO | VM_DONTDUMP |
> +					VM_PFNMAP | VM_SEALED_SYSMAP,
>  					&vdso_vvar_mapping);
>  }
>
> --
> 2.45.2
>

Re: [PATCH mm-unstable 1/2] mseal sysmap: generic vdso vvar mapping

[Jeff Xu]

On Tue, Mar 11, 2025 at 5:33 AM Heiko Carstens <hca@linux.ibm.com> wrote:
>
> With the introduction of the generic vdso data storage the VM_SEALED_SYSMAP
> vm flag must be moved from the architecture specific
> _install_special_mapping() call [1] [2] which maps the vvar mapping to
> generic code.
>
Thanks for fixing this merging problem.
The new selftest in mseal_system_mappings should catch this :-)

Hi Andrew,
when you apply this to mm-unstable, could you please insert this between:

commit 5bda54a4f304a7c3d65a40c9c3f015901caa2ce6
selftests: x86: test_mremap_vdso: skip if vdso is msealed
and
commit 884323e928c6938923e5dfcb8a601b3363e1130b
 mseal sysmap: enable x86-64

That will make the series complete.


> [1] https://lkml.kernel.org/r/20250305021711.3867874-4-jeffxu@google.com
> [2] https://lkml.kernel.org/r/20250305021711.3867874-5-jeffxu@google.com
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  lib/vdso/datastore.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/vdso/datastore.c b/lib/vdso/datastore.c
> index e227fbbcb796..b7c7386c98a9 100644
> --- a/lib/vdso/datastore.c
> +++ b/lib/vdso/datastore.c
> @@ -99,7 +99,8 @@ const struct vm_special_mapping vdso_vvar_mapping = {
>  struct vm_area_struct *vdso_install_vvar_mapping(struct mm_struct *mm, unsigned long addr)
>  {
>         return _install_special_mapping(mm, addr, VDSO_NR_PAGES * PAGE_SIZE,
> -                                       VM_READ | VM_MAYREAD | VM_IO | VM_DONTDUMP | VM_PFNMAP,
> +                                       VM_READ | VM_MAYREAD | VM_IO | VM_DONTDUMP |
> +                                       VM_PFNMAP | VM_SEALED_SYSMAP,
>                                         &vdso_vvar_mapping);
>  }
>
> --
> 2.45.2
>
Reviewed-by: Jeff Xu <jeffxu@chromium.org>

[PATCH mm-unstable 2/2] mseal sysmap: enable s390

[Heiko Carstens]

Provide support for CONFIG_MSEAL_SYSTEM_MAPPINGS on s390, covering the
vdso.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/Kconfig       | 1 +
 arch/s390/kernel/vdso.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 608a965e2344..93b880992596 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -141,6 +141,7 @@ config S390
 	select ARCH_SUPPORTS_DEBUG_PAGEALLOC
 	select ARCH_SUPPORTS_HUGETLBFS
 	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && CC_IS_CLANG
+	select ARCH_SUPPORTS_MSEAL_SYSTEM_MAPPINGS
 	select ARCH_SUPPORTS_NUMA_BALANCING
 	select ARCH_SUPPORTS_PER_VMA_LOCK
 	select ARCH_USE_BUILTIN_BSWAP
diff --git a/arch/s390/kernel/vdso.c b/arch/s390/kernel/vdso.c
index 70c8f9ad13cd..430feb1a5013 100644
--- a/arch/s390/kernel/vdso.c
+++ b/arch/s390/kernel/vdso.c
@@ -80,7 +80,7 @@ static int map_vdso(unsigned long addr, unsigned long vdso_mapping_len)
 	vdso_text_start = vvar_start + VDSO_NR_PAGES * PAGE_SIZE;
 	/* VM_MAYWRITE for COW so gdb can set breakpoints */
 	vma = _install_special_mapping(mm, vdso_text_start, vdso_text_len,
-				       VM_READ|VM_EXEC|
+				       VM_READ|VM_EXEC|VM_SEALED_SYSMAP|
 				       VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
 				       vdso_mapping);
 	if (IS_ERR(vma)) {
-- 
2.45.2

Re: [PATCH mm-unstable 2/2] mseal sysmap: enable s390

[Lorenzo Stoakes]

On Tue, Mar 11, 2025 at 01:33:26PM +0100, Heiko Carstens wrote:
> Provide support for CONFIG_MSEAL_SYSTEM_MAPPINGS on s390, covering the
> vdso.
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  arch/s390/Kconfig       | 1 +
>  arch/s390/kernel/vdso.c | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 608a965e2344..93b880992596 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -141,6 +141,7 @@ config S390
>  	select ARCH_SUPPORTS_DEBUG_PAGEALLOC
>  	select ARCH_SUPPORTS_HUGETLBFS
>  	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && CC_IS_CLANG
> +	select ARCH_SUPPORTS_MSEAL_SYSTEM_MAPPINGS

As discussed in 0/2, s390 does not rely on remapping system mappings in any
kind of bizarre way so this is all good.

>  	select ARCH_SUPPORTS_NUMA_BALANCING
>  	select ARCH_SUPPORTS_PER_VMA_LOCK
>  	select ARCH_USE_BUILTIN_BSWAP
> diff --git a/arch/s390/kernel/vdso.c b/arch/s390/kernel/vdso.c
> index 70c8f9ad13cd..430feb1a5013 100644
> --- a/arch/s390/kernel/vdso.c
> +++ b/arch/s390/kernel/vdso.c
> @@ -80,7 +80,7 @@ static int map_vdso(unsigned long addr, unsigned long vdso_mapping_len)
>  	vdso_text_start = vvar_start + VDSO_NR_PAGES * PAGE_SIZE;
>  	/* VM_MAYWRITE for COW so gdb can set breakpoints */
>  	vma = _install_special_mapping(mm, vdso_text_start, vdso_text_len,
> -				       VM_READ|VM_EXEC|
> +				       VM_READ|VM_EXEC|VM_SEALED_SYSMAP|
>  				       VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
>  				       vdso_mapping);
>  	if (IS_ERR(vma)) {
> --
> 2.45.2
>

Re: [PATCH mm-unstable 2/2] mseal sysmap: enable s390

[Jeff Xu]

On Tue, Mar 11, 2025 at 5:33 AM Heiko Carstens <hca@linux.ibm.com> wrote:
>
> Provide support for CONFIG_MSEAL_SYSTEM_MAPPINGS on s390, covering the
> vdso.
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/Kconfig       | 1 +
>  arch/s390/kernel/vdso.c | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 608a965e2344..93b880992596 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -141,6 +141,7 @@ config S390
>         select ARCH_SUPPORTS_DEBUG_PAGEALLOC
>         select ARCH_SUPPORTS_HUGETLBFS
>         select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && CC_IS_CLANG
> +       select ARCH_SUPPORTS_MSEAL_SYSTEM_MAPPINGS
>         select ARCH_SUPPORTS_NUMA_BALANCING
>         select ARCH_SUPPORTS_PER_VMA_LOCK
>         select ARCH_USE_BUILTIN_BSWAP
> diff --git a/arch/s390/kernel/vdso.c b/arch/s390/kernel/vdso.c
> index 70c8f9ad13cd..430feb1a5013 100644
> --- a/arch/s390/kernel/vdso.c
> +++ b/arch/s390/kernel/vdso.c
> @@ -80,7 +80,7 @@ static int map_vdso(unsigned long addr, unsigned long vdso_mapping_len)
>         vdso_text_start = vvar_start + VDSO_NR_PAGES * PAGE_SIZE;
>         /* VM_MAYWRITE for COW so gdb can set breakpoints */
>         vma = _install_special_mapping(mm, vdso_text_start, vdso_text_len,
> -                                      VM_READ|VM_EXEC|
> +                                      VM_READ|VM_EXEC|VM_SEALED_SYSMAP|
>                                        VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
>                                        vdso_mapping);
>         if (IS_ERR(vma)) {
> --
> 2.45.2
>
Great ! one more architecture uses this feature.

Reviewed-by: Jeff Xu <jeffxu@chromium.org>

Re: [PATCH mm-unstable 2/2] mseal sysmap: enable s390

[Jeff Xu]

One more thing: do you want to update mseal.rst to add s390 as
supported architecture?

Currently in the mseal.rst:
 "The following architectures currently support this feature: x86-64 and arm64."

Thanks
-Jeff

On Tue, Mar 11, 2025 at 2:55 PM Jeff Xu <jeffxu@chromium.org> wrote:
>
> On Tue, Mar 11, 2025 at 5:33 AM Heiko Carstens <hca@linux.ibm.com> wrote:
> >
> > Provide support for CONFIG_MSEAL_SYSTEM_MAPPINGS on s390, covering the
> > vdso.
> >
> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> > ---
> >  arch/s390/Kconfig       | 1 +
> >  arch/s390/kernel/vdso.c | 2 +-
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> > index 608a965e2344..93b880992596 100644
> > --- a/arch/s390/Kconfig
> > +++ b/arch/s390/Kconfig
> > @@ -141,6 +141,7 @@ config S390
> >         select ARCH_SUPPORTS_DEBUG_PAGEALLOC
> >         select ARCH_SUPPORTS_HUGETLBFS
> >         select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && CC_IS_CLANG
> > +       select ARCH_SUPPORTS_MSEAL_SYSTEM_MAPPINGS
> >         select ARCH_SUPPORTS_NUMA_BALANCING
> >         select ARCH_SUPPORTS_PER_VMA_LOCK
> >         select ARCH_USE_BUILTIN_BSWAP
> > diff --git a/arch/s390/kernel/vdso.c b/arch/s390/kernel/vdso.c
> > index 70c8f9ad13cd..430feb1a5013 100644
> > --- a/arch/s390/kernel/vdso.c
> > +++ b/arch/s390/kernel/vdso.c
> > @@ -80,7 +80,7 @@ static int map_vdso(unsigned long addr, unsigned long vdso_mapping_len)
> >         vdso_text_start = vvar_start + VDSO_NR_PAGES * PAGE_SIZE;
> >         /* VM_MAYWRITE for COW so gdb can set breakpoints */
> >         vma = _install_special_mapping(mm, vdso_text_start, vdso_text_len,
> > -                                      VM_READ|VM_EXEC|
> > +                                      VM_READ|VM_EXEC|VM_SEALED_SYSMAP|
> >                                        VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
> >                                        vdso_mapping);
> >         if (IS_ERR(vma)) {
> > --
> > 2.45.2
> >
> Great ! one more architecture uses this feature.
>
> Reviewed-by: Jeff Xu <jeffxu@chromium.org>

Re: [PATCH mm-unstable 2/2] mseal sysmap: enable s390

[Heiko Carstens]

On Tue, Mar 11, 2025 at 03:12:55PM -0700, Jeff Xu wrote:
> One more thing: do you want to update mseal.rst to add s390 as
> supported architecture?
> 
> Currently in the mseal.rst:
>  "The following architectures currently support this feature: x86-64 and arm64."

Thanks, I missed that, however I'm wondering if it would make sense to add
mseal to the generic feature list instead (no, I am not volunteering to do
that :) ).

See Documentation/features/... and scripts/get_feat.pl

Re: [PATCH mm-unstable 2/2] mseal sysmap: enable s390

[Jeff Xu]

On Wed, Mar 12, 2025 at 8:39 AM Heiko Carstens <hca@linux.ibm.com> wrote:
>
> On Tue, Mar 11, 2025 at 03:12:55PM -0700, Jeff Xu wrote:
> > One more thing: do you want to update mseal.rst to add s390 as
> > supported architecture?
> >
> > Currently in the mseal.rst:
> >  "The following architectures currently support this feature: x86-64 and arm64."
>
> Thanks, I missed that, however I'm wondering if it would make sense to add
> mseal to the generic feature list instead (no, I am not volunteering to do
> that :) ).
>
> See Documentation/features/... and scripts/get_feat.pl
Thanks for this suggestion. I will add that file.

-Jeff

Re: [PATCH mm-unstable 0/2] mseal system mappings fix + s390 enablement

[Lorenzo Stoakes]

On Tue, Mar 11, 2025 at 01:33:24PM +0100, Heiko Carstens wrote:
> When rebasing the mseal series on top of the generic vdso data storage
> the VM_SEALED_SYSMAP vm flag for the vvar mapping got lost. Add that again.

I'm confused by this? Some merge patch resolution thing?

I don't think this should be at the top of a description of a series, seems more
like an addendum.

>
> Also add s390 support for MSEAL_SYSTEM_MAPPINGS.

'Also' = the whole thing this series does?

Can you confirm that s390 absolutely does not rely upon
moving/manipulating/etc. the VDSO, VVAR, etc. mappings?

You should say that here.

>
> Heiko Carstens (2):
>   mseal sysmap: generic vdso vvar mapping
>   mseal sysmap: enable s390
>
>  arch/s390/Kconfig       | 1 +
>  arch/s390/kernel/vdso.c | 2 +-
>  lib/vdso/datastore.c    | 3 ++-
>  3 files changed, 4 insertions(+), 2 deletions(-)
>
> --
> 2.45.2
>

Re: [PATCH mm-unstable 0/2] mseal system mappings fix + s390 enablement

[Heiko Carstens]

On Tue, Mar 11, 2025 at 01:02:47PM +0000, Lorenzo Stoakes wrote:
> On Tue, Mar 11, 2025 at 01:33:24PM +0100, Heiko Carstens wrote:
> > When rebasing the mseal series on top of the generic vdso data storage
> > the VM_SEALED_SYSMAP vm flag for the vvar mapping got lost. Add that again.
> 
> I'm confused by this? Some merge patch resolution thing?

See my other mail.

> > Also add s390 support for MSEAL_SYSTEM_MAPPINGS.
> 
> 'Also' = the whole thing this series does?
> 
> Can you confirm that s390 absolutely does not rely upon
> moving/manipulating/etc. the VDSO, VVAR, etc. mappings?
> 
> You should say that here.

Just like for arm64, and x86_64 the s390 part is just adding the new
vm flag to the _install_mappings() call in vdso code. Otherwise there
is nothing to be considered.

Re: [PATCH mm-unstable 0/2] mseal system mappings fix + s390 enablement

[Lorenzo Stoakes]

On Tue, Mar 11, 2025 at 02:37:36PM +0100, Heiko Carstens wrote:
> On Tue, Mar 11, 2025 at 01:02:47PM +0000, Lorenzo Stoakes wrote:
> > On Tue, Mar 11, 2025 at 01:33:24PM +0100, Heiko Carstens wrote:
> > > When rebasing the mseal series on top of the generic vdso data storage
> > > the VM_SEALED_SYSMAP vm flag for the vvar mapping got lost. Add that again.
> >
> > I'm confused by this? Some merge patch resolution thing?
>
> See my other mail.
>
> > > Also add s390 support for MSEAL_SYSTEM_MAPPINGS.
> >
> > 'Also' = the whole thing this series does?
> >
> > Can you confirm that s390 absolutely does not rely upon
> > moving/manipulating/etc. the VDSO, VVAR, etc. mappings?
> >
> > You should say that here.
>
> Just like for arm64, and x86_64 the s390 part is just adding the new
> vm flag to the _install_mappings() call in vdso code. Otherwise there
> is nothing to be considered.

No, they are not just adding a flag, they are enabling the sealing of
system mappings, if a user chooses to make use of it, which already breaks
a number of useland applications that rely on remapping these.

if the architecture code ever needs to unmap/remap these, then this breaks
your architecture.

I think it would be sensible to clearly indicate that enabling this feature
does not break the s390 architecture and you've confirmed that by checking
the code and ensuring that nowhere does it rely upon doing this.

Likely that's the case, but I'd suggest you ought to make sure...

x86-64 and arm64 were checked for this and confirmed to not ever need this.

This is why we're restricting by architecture.

Re: [PATCH mm-unstable 0/2] mseal system mappings fix + s390 enablement

[Heiko Carstens]

On Tue, Mar 11, 2025 at 01:42:01PM +0000, Lorenzo Stoakes wrote:
> > Just like for arm64, and x86_64 the s390 part is just adding the new
> > vm flag to the _install_mappings() call in vdso code. Otherwise there
> > is nothing to be considered.
> 
> No, they are not just adding a flag, they are enabling the sealing of
> system mappings, if a user chooses to make use of it, which already breaks
> a number of useland applications that rely on remapping these.
> 
> if the architecture code ever needs to unmap/remap these, then this breaks
> your architecture.
> 
> I think it would be sensible to clearly indicate that enabling this feature
> does not break the s390 architecture and you've confirmed that by checking
> the code and ensuring that nowhere does it rely upon doing this.
> 
> Likely that's the case, but I'd suggest you ought to make sure...
> 
> x86-64 and arm64 were checked for this and confirmed to not ever need this.
> 
> This is why we're restricting by architecture.

Ok, I was assuming more that whoever enables that config option knows
what he or she is doing. However as far as I know there is no s390
user space which relies on remapping vdso mappings.

When it comes to unmapping vdso: this would break user space since
commit df29a7440c4b ("s390/signal: switch to using vdso for sigreturn
and syscall restart") - there haven't been any reports.

Re: [PATCH mm-unstable 0/2] mseal system mappings fix + s390 enablement

[Lorenzo Stoakes]

On Tue, Mar 11, 2025 at 03:06:30PM +0100, Heiko Carstens wrote:
> On Tue, Mar 11, 2025 at 01:42:01PM +0000, Lorenzo Stoakes wrote:
> > > Just like for arm64, and x86_64 the s390 part is just adding the new
> > > vm flag to the _install_mappings() call in vdso code. Otherwise there
> > > is nothing to be considered.
> >
> > No, they are not just adding a flag, they are enabling the sealing of
> > system mappings, if a user chooses to make use of it, which already breaks
> > a number of useland applications that rely on remapping these.
> >
> > if the architecture code ever needs to unmap/remap these, then this breaks
> > your architecture.
> >
> > I think it would be sensible to clearly indicate that enabling this feature
> > does not break the s390 architecture and you've confirmed that by checking
> > the code and ensuring that nowhere does it rely upon doing this.
> >
> > Likely that's the case, but I'd suggest you ought to make sure...
> >
> > x86-64 and arm64 were checked for this and confirmed to not ever need this.
> >
> > This is why we're restricting by architecture.
>
> Ok, I was assuming more that whoever enables that config option knows
> what he or she is doing. However as far as I know there is no s390
> user space which relies on remapping vdso mappings.

Yeah, we allow for the 'user knows what they're doing' stuff when it comes to
_userland_, but we obviously don't want to ship a known-broken kernel :)

>
> When it comes to unmapping vdso: this would break user space since
> commit df29a7440c4b ("s390/signal: switch to using vdso for sigreturn
> and syscall restart") - there haven't been any reports.

OK that seems to implicitly suggest that you're fine with sealing then?

I had a quick glance and it seemed fine for s390. I mean it's _weird_ to remap
any of this stuff so it'd be the odd one out that does it (ppc _seems_ to in
_some_ circumstances need it, for instance).

So I think we're good? :)

Re: [PATCH mm-unstable 0/2] mseal system mappings fix + s390 enablement

[Heiko Carstens]

On Tue, Mar 11, 2025 at 02:09:30PM +0000, Lorenzo Stoakes wrote:
> > When it comes to unmapping vdso: this would break user space since
> > commit df29a7440c4b ("s390/signal: switch to using vdso for sigreturn
> > and syscall restart") - there haven't been any reports.
> 
> OK that seems to implicitly suggest that you're fine with sealing then?
> 
> I had a quick glance and it seemed fine for s390. I mean it's _weird_ to remap
> any of this stuff so it'd be the odd one out that does it (ppc _seems_ to in
> _some_ circumstances need it, for instance).
> 
> So I think we're good? :)

Yes, for s390 we are good.

Re: [PATCH mm-unstable 0/2] mseal system mappings fix + s390 enablement

[Lorenzo Stoakes]

On Tue, Mar 11, 2025 at 03:21:14PM +0100, Heiko Carstens wrote:
> On Tue, Mar 11, 2025 at 02:09:30PM +0000, Lorenzo Stoakes wrote:
> > > When it comes to unmapping vdso: this would break user space since
> > > commit df29a7440c4b ("s390/signal: switch to using vdso for sigreturn
> > > and syscall restart") - there haven't been any reports.
> >
> > OK that seems to implicitly suggest that you're fine with sealing then?
> >
> > I had a quick glance and it seemed fine for s390. I mean it's _weird_ to remap
> > any of this stuff so it'd be the odd one out that does it (ppc _seems_ to in
> > _some_ circumstances need it, for instance).
> >
> > So I think we're good? :)
>
> Yes, for s390 we are good.

Awesome :) series otherwise looks good.