Re: [PATCH] Revert "evtchn: refuse EVTCHNOP_status for Xen-bound event channels"

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 16 May 2024, Jan Beulich wrote:
> 1) In the discussion George claimed that exposing status information in
> an uncontrolled manner is okay. I'm afraid I have to disagree, seeing
> how a similar assumption by CPU designers has led to a flood of
> vulnerabilities over the last 6+ years. Information exposure imo is never
> okay, unless it can be _proven_ that absolutely nothing "useful" can be
> inferred from it. (I'm having difficulty seeing how such a proof might
> look like.)

Many would agree that it is better not to expose status information in
an uncontrolled manner. Anyway, let's focus on the actionable.


> 2) Me pointing out that the XSM hook might similarly get in the way of
> debugging, Andrew suggested that this is not an issue because any sensible
> XSM policy used in such an environment would grant sufficient privilege to
> Dom0. Yet that then still doesn't cover why DomU-s also can obtain status
> for Xen-internal event channels. The debugging argument then becomes weak,
> as in that case the XSM hook is possibly going to get in the way.
>
> 3) In the discussion Andrew further gave the impression that evtchn_send()
> had no XSM check. Yet it has; the difference to evtchn_status() is that
> the latter uses XSM_TARGET while the former uses XSM_HOOK. (Much like
> evtchn_status() may indeed be useful for debugging, evtchn_send() may be
> similarly useful to allow getting a stuck channel unstuck.)
> 
> In summary I continue to think that an outright revert was inappropriate.
> DomU-s should continue to be denied status information on Xen-internal
> event channels, unconditionally and independent of whether dummy, silo, or
> Flask is in use.

I think DomU-s should continue to be denied status information on
Xen-internal event channels *based on the default dummy, silo, or Flask
policy*. It is not up to us to decide the security policy, only to
enforce it and provide sensible defaults.

In any case, the XSM_TARGET check in evtchn_status seems to do what we
want?

evtchn_send uses XSM_HOOK, which is weaker, but it doesn't seem to be an
issue because (ignoring the consumer_is_xen check) there is a if(!lchn)
check that would fail on invalid event channels?


> Plus, as stated before, evtchn_send() would better remain in sync in this
> regard with evtchn_status(). The situation is less clear for evtchn_close()
> and evtchn_bind_vcpu(): Those indeed have no XSM checks while they do deny
> operation on Xen-internal channels. It is likely more far fetched to
> assume permitting them for debugging purposes might in fact help in rare
> situations. Yet it may still be a matter of consistency to keep them in
> sync with the other two. (Note that there's also evtchn_usable(), which
> might then also need tweaking itself or at the use sites.)
> 
> FTR, it is going to be only then that I would consider the cumulative
> result as eligible for backporting. For this purpose, at the risk of
> being flamed again, it might still be easier to revert the revert and then
> put in place a change meeting the above criteria. That could then be taken
> for backport as is.

I think we want to keep the revert because we need to unbreak lsevtchn.c.
Your point about consistency is valid and it would be good to go in that
direction but to me it is not the kind of thing that we would make
release-blocking.