Re: [PATCH v4 1/1] exec: seal system mappings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kees,

I reply inline below but the TL;DR is - I'm fine with an incremental
approach, my requirements for arch support are sensible and doable and I'll
_give a R-b tag_ if such a version is submitted. :)

This isn't a discreet means of me trying to reject the whole idea, if I
felt that way I'd just say!

I actually firmly want to _help_ mseal features land in the kernel.

Cheers, Lorenzo

On Wed, Jan 15, 2025 at 03:52:23PM -0800, Kees Cook wrote:
> On Wed, Jan 15, 2025 at 07:46:00PM +0000, Lorenzo Stoakes wrote:
> > You are now suggesting disabling the !CRIU requirement. Which violates my
> > _requirements_ (not optional features).
>
> Why not make this simply incremental? The feature isn't intended to work
> with CRIU. Why don't we get the feature in first, with a !CRUI depends?
> This lets the users of this feature actually use it.

Sure, I'm ok with this.

The analysis at the end of the series suggested the consensus was otherwise,
which is why I highlighted this.

>
> > You seem to be saying you're pushing an internal feature on upstream and
> > only care about internal use cases, this is not how upstream works, as
> > Matthew alludes to.
>
> Internal? No. Chrome OS and Android. Linux runs more Android devices
> than everything else in the world combined -- this is not some random
> experiment.

This is unfair, I'm not claiming otherwise, and I would suggest you look
into other work I've done which has directly benefitted android if you
believe I'm not aware of how widespread a user it is (you're welcome ;) I
also own and very much enjoy my Pixel Pro 9 Fold 2...

I'm saying we can't _only_ consider this. This is upstream kernel, we must
consider all architectures and use cases.

This seems a reasonable position to me.

>
> I really don't like the feature creep nature of the system mapping
> sealing reviews. There's nothing special here -- we have plenty of
> features that conflict with other features. And we have a long history
> in the kernel of landing the core changes with lots of conflict depends
> that we then resolve as we move forward.

There's been no feature creep. I explicitly said very early on what the
problem was and what needed to be done to fix it.

Then a bunch of discussion happened and an analysis was presented that
seemed to neglect this.

I also don't agree we have a long history of landing changes that quietly
break things in the kernel.

As I said in the mail you are responding to - my concern is that a kernel
user will enable this feature, not realising that it breaks X, Y or Z, and
there's no easy or clear way for them to know.

This was originally addressed with config flags, but then boot options were
provided which completely overrode this.

My concern is there is a disconnect between a kernel user seeing a security
feature - and them knowing or realising that it is broken, for instance, if
you try to use CRIU.

Then suddenly what seems a reasonable feature to enable is suddenly a
landmine for somebody to step on to break their system.

Again, I have no objection to a version of this series which explicitly
disallows known-broken scenarios.

>
> Why not just make system map sealing conflict with CRIU? Who is asking
> to use both at the same time?

Again, the analysis Jeff presented appeared to rule out doing this. Hence
my objection.

If we explicitly disallow this (with no boot override) I'm fine with it!

Sorry if I wasn't clear about that.

>
> > I have told you that my requirements are:
> >
> > 1. You cannot allow a user to set config or boot options to have a
> >    broken kernel configuration.
>
> What do you define as a "broken kernel configuration"?

One which results in a kernel which cannot function correctly in some
fundamental respect. For instance, breaking userspace for certain programs
when a feature which might non-obviously do so.

>
> > 2. You must provide evidence that the arches you claim work with this,
> >    actually do.
>
> What evidence would you find sufficient? I'm concerned this is turning
> into a rock fetching quest.

That no code relies on the VDSO being non-sealed in supported raches, you
can do this by pointing out the code that interacts with the VDSO in these
arches in all instances does not encounter a problem when this is so.

I _believe_ this is a reasonable request, and sort of a fundamental thing
you'd want for a change like this for such a weird beast as the VDSO.

>
> -Kees
>
> --
> Kees Cook

Cheers, Lorenzo




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux