Re: [PATCH] xsm: also panic upon "flask=enforcing" when XSM_FLASK=n

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.05.2020 12:30, Jan Beulich wrote:
> On 29.05.2020 12:07, Andrew Cooper wrote:
>> On 29/05/2020 10:34, Jan Beulich wrote:
>>> While the behavior to ignore this option without FLASK support was
>>> properly documented, it is still somewhat surprising to someone using
>>> this option and then still _not_ getting the assumed security. Add a
>>> 2nd handler for the command line option for the XSM_FLASK=n case, and
>>> invoke panic() when the option is specified (and not subsequently
>>> overridden by "flask=disabled").
>>>
>>> Suggested-by: Ian Jackson <ian.jackson@xxxxxxxxxx>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> I'm very tempted to nack this outright, lest I remind both of you of the
>> total disaster that was XSA-9, and the subsequent retraction of the code
>> which did exactly this.
>>
>> If you want to do something like this, prohibit creating guests so the
>> administrator can still log in and unbreak,
> 
> Unbreaking is as easy as removing the command line option, or
> adding "flask=disable" at the end of the command line.
> 
> Preventing to create guests is another option, but complicated
> by the late-hwdom feature we still have - to achieve what you
> want we'd have to permit creating that one further domain.
> Dom0less perhaps also would need special treatment (and there
> I'm not sure we'd know which of the domains we are supposed to
> allow being created, and which one(s) not).

Furthermore the policy that would normally be loaded might
constrain Dom0 itself as well. Allowing Dom0 to boot is therefore
not necessarily the right thing to do. Therefore, while overall I
agree that generalizing x86's "allow_unsafe" may be a helpful
thing to do, it's not what I would want to use here. Instead,
together with ...

>> instead of having it
>> entering a reboot loop with no output.  The console isn't established
>> this early, so none of this text makes it out onto VGA/serial.
> 
> You didn't look at the patch then: I'm intentionally _not_
> panic()-ing from the command line parsing function, but from
> an initcall. Both VGA and serial have been set up by that time.
> (I was in fact considering to pull it a little earlier, into
> a pre-SMP initcall.)

... this, I think the patch wants to be re-considered as is.

Jan