>>> On 02.07.18 at 11:22, <[email protected]> wrote:
>> From: Jan Beulich [mailto:[email protected]]
>> Sent: Monday, July 2, 2018 3:29 PM
>> >>> On 02.07.18 at 08:57, <[email protected]> wrote:
>> >> From: Jan Beulich [mailto:[email protected]]
>> >> Sent: Friday, June 29, 2018 6:36 PM
>> >> >>> On 29.06.18 at 11:28, <[email protected]> wrote:
>> >> > When SILO is enabled, there would be no page-sharing between
>> >> > unprivileged VMs (no grant tables or event channels).
>> >>
>> >> What is the relation between page sharing and event channels?
>> >
>> > They are the two mechanisms exist for inter-domain communication, And
>> > we want to block them in SILO mode.
>>
>> I understand this, but it doesn't answer my question. I agree that grant
>> tables
>> are a means to share pages, but the wording looks odd to me wrt event
>> channels.
> Do you mean add " or event notifications",
> When SILO is enabled, there would be no page-sharing or event notifications
> between unprivileged VMs (no grant tables or event channels).
Yes, that's one way to clarify things.
>> > Change to:
>> >
>> > config XSM_SILO
>> >>-------def_bool y
>> >>-------prompt "SILO support"
>> >>-------depends on XSM
>> >>----------help---
>> >>------- Enables SILO as the access control mechanism used by the XSM
>> framework.
>> >>------- This is not the default module, add boot parameter xsm=silo
>> >>to choose
>> >>------- it. This will deny any unmediated communication channels
>> >>(grant tables
>> >>------- and event channels) between unprivileged VMs.
>>
>> With s/module/mode/ this is an improvement, but continues to leave open in
>> particular what an "unmediated communication channel" is.
> This can't prevent side-channel attack.
???
>> Btw, thinking about it again - do we need a Kconfig option here in the first
>> place, when the mode isn't the default, and it's not a whole lot of code
> that gets
>> added?
> The existing XSM code use Kconfig,
> I just want to follow the similar style for new module.
> And yes, we can handle it in CONFIG_XSM like dummy.
> Which way is better?
Daniel, Andrew?
>> >> > +static bool silo_mode_dom_check(domid_t ldom, domid_t rdom) {
>> >> > + domid_t hd_dom = hardware_domain->domain_id;
>> >>
>> >> I don't think you mean the hardware domain here, but the control
>> >> domain (of which in theory there may be multiple).
>> >
>> > I mean the one and only dom0.
>>
>> No, for the purpose here you don't mean Dom0, which just happens to be both
>> hardware and (the only) control domain in most setups. From a security pov
>> though you need to distinguish all of these.
>
> Yes! thanks.
> I will use
> is_control_domain(d)
> instead of comparing the hardware domain id.
>
> This comment is misleading then:
> /* Is this guest fully privileged (aka dom0)? */
> bool is_privileged;
Yes, but it's likely going to remain that way until further disaggregation
work would happen.
>> >> > + domid_t cur_dom = current->domain->domain_id;
>> >> > +
>> >> > + if ( ldom == DOMID_SELF )
>> >> > + ldom = cur_dom;
>> >> > + if ( rdom == DOMID_SELF )
>> >> > + rdom = cur_dom;
>> >> > +
>> >> > + return (hd_dom == cur_dom || hd_dom == ldom || hd_dom == rdom
>> ||
>> >> > + ldom == rdom);
>> >> > +}
>> >> > +
>> >> > +static int silo_evtchn_unbound(struct domain *d1, struct evtchn *chn,
>> >> > + domid_t id2) {
>> >> > + if ( silo_mode_dom_check(d1->domain_id, id2) )
>> >> > + return dummy_xsm_ops.evtchn_unbound(d1, chn, id2);
>> >>
>> >> Urgh. Why is this not xsm_evtchn_unbound() from dummy.h? It would be
>> >> really nice to avoid such extra indirect calls here.
>> >
>> > This makes it clearer that we are calling the counterpart of dummy
>> > ops(overriding).
>>
>> Yes, but the same level of clarity could be achieved when naming the function
>> in dummy.h dummy_evtchn_unbound() (aliased to
>> xsm_evtchn_unbound() for satisfying needs elsewhere).
>>
>> > This indirect calls should not introduce any runtime penalty.
>>
>> How does it not, when indirect calls are more expensive than direct ones
>> already without the Spectre v2 mitigations?
> I only mean it's not runtime binding.
>
> And I ran some performance test before, seems no performance penalty.
Sure, these paths aren't normally performance critical. But by doing what
you do we'd have a bad precedent, and if someone later cloned your
solution into something that does sit on a performance critical path, we'd
have a problem.
> The names in dummy.h are the same as xsm.h.
> If I call xsm_evtchn_unbound, that's from xsm.h,
> It probably call silo_evtchn_unbound, ends up in a loop.
>
> So I may have to rename all the functions in dummy.h,
Note how I've said "naming the function in dummy.h
dummy_evtchn_unbound() (aliased to xsm_evtchn_unbound()
for satisfying needs elsewhere)."
> And remove static...
I don't think so, no.
> is it necessary?
It should imo be at least considered. But Daniel as the maintainer may
have something to say here as well...
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
