| Message ID | 20250311123326.2686682-1-hca@linux.ibm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mseal system mappings fix + s390 enablement | expand |
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 >
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.
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.
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.
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? :)
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.
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.