On Tue, Feb 25, 2025 at 07:06:13AM -0800, Kees Cook wrote: > > > On February 25, 2025 2:37:11 AM PST, Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> wrote: > >On Tue, Feb 25, 2025 at 08:45:21AM +0000, Berg, Benjamin wrote: > >> Hi, > >> > >> On Tue, 2025-02-25 at 06:22 +0000, Lorenzo Stoakes wrote: > >> > On Mon, Feb 24, 2025 at 10:52:44PM +0000, jeffxu@xxxxxxxxxxxx wrote: > >> > > From: Jeff Xu <jeffxu@xxxxxxxxxxxx> > >> > > > >> > > Provide support for CONFIG_MSEAL_SYSTEM_MAPPINGS on UML, covering > >> > > the vdso. > >> > > > >> > > Testing passes on UML. > >> > > >> > Maybe expand on this by stating that it has been confirmed by Benjamin (I > >> > _believe_) that UML has no need for problematic relocation so this is known to > >> > be good. > >> > >> I may well be misreading this message, but this sounds to me that this > >> is a misinterpretation. So, just to clarify in case that is needed. > >> > >> CONFIG_MSEAL_SYSTEM_MAPPINGS does work fine for the UML kernel. > >> However, the UML kernel is a normal userspace application itself and > >> for this application to run, the host kernel must have the feature > >> disabled. > >> > >> So, UML supports the feature. But it still *cannot* run on a host > >> machine that has the feature enabled. > > > >Sigh ok. Apologies if I misunderstood. > > > >Is there any point having this for the 'guest' system? I mean security wise are > >we concerned about sealing of system mappings? > > UML guests are used for testing. For example, it's the default target for KUnit's scripts. Having sealing working in the guest seems generally useful to me. > 'Having sealing working' you mean system sealing? Because mseal works fine (presumably in UML, not tried myself!) System msealing lacks any test in this series (I did ask for them...), certainly no kunit tests, so this seems a bit theoretical? Unless you're talking about the theoretical interaction of kunit tests and VDSO sealing? I mean can't we just introduce this at the time if we believe this'd be useful? Adding stuff for entirely theoretical reasons is generally frowned upon and isn't that the (at least one) definition of churn? Can you give a really specific reason why this needs to be added? For x86-64, arm64 the reason is obvious (albeit one can argue how much security benefit msealing the VDSO truly gives). For UML it just isn't, at all. > > > >I feel like having this here might just add confusion and churn if it's not > >useful. > > > >If this is useless for UML guest, let's just drop this patch. > > But on the flip side, it's certainly not critical to have UML supported. I guess I just don't see a down side to keeping the patch. Right, presumably you disagree that it might be confusing that to test mseal _system_ sealing (not mseal itself) by enabling mseal system sealing in UML but having to disable it in the host? Because that seems really confusing to me (and why I explicitly cited it as a reason I'm confused here, even the exchange above is confused). If there benefit is basically theoretical/why not/nil, and the downside is confusion + churn (albeit small churn), why not just wait until somebody writes kunit tests? If I saw a list of supported arches including UML, but also saw a note about it not working in UML I'd definitely be confused. Generally I'm not a fan of adding features mid-way through a series, the revisions are meant to be refinements of the original, not an evolving thing. So in general I'd prefer this to be added if + when we need it for something. > > -Kees > > > -- > Kees Cook
