On 31.08.2021 17:25, Andrew Cooper wrote: > On 31/08/2021 14:26, Jan Beulich wrote: >> On 31.08.2021 15:16, Andrew Cooper wrote: >>> On 30/08/2021 14:02, Jan Beulich wrote: >>>> Further permit "access" to differ in the "executable" attribute. While >>>> ideally only ROM regions would get mapped with X set, getting there is >>>> quite a bit of work. Therefore, as a temporary measure, permit X to >>>> vary. For Dom0 the more permissive of the types will be used, while for >>>> DomU it'll be the more restrictive one. >>> Split behaviour between dom0 and domU based on types alone cannot >>> possibly be correct. >> True, but what do you do. >> >>> DomU's need to execute ROMs too, and this looks like will malfunction if >>> a ROM ends up in the region that HVMLoader relocated RAM from. >>> >>> As this is a temporary bodge emergency bugfix, don't try to be clever - >>> just take the latest access. >> And how do we know that that's what is going to work? > > Because it's the pre-existing behaviour.
Valid point. But for the DomU case there simply has not been any pre-existing behavior. Hence my desire to be restrictive initially there. >> We should >> strictly accumulate for Dom0. And what we do for DomU is moot for >> the moment, until PCI passthrough becomes a thing for PVH. Hence >> I've opted to be restrictive there - I'd rather see things break >> (and getting adjusted) when this future work actually gets carried >> out, than leave things permissive for no-one to notice that it's >> too permissive, leading to an XSA. > > Restricting execute permissions is something unique to virt. It doesn't > exist in a non-virtualised system, as I and D side reads are > indistinguishable outside of the core. > > Furthermore, it is inexpressible on some systems/configurations. > > Introspection is the only technology which should be restricting execute > permissions in the p2m, and only when it takes responsibility for > dealing with the fallout. IOW are you saying that the calls to set_identity_p2m_entry() (pre-dating XSA-378) were wrong to use p2m_access_rw? Because that's what's getting the way here. Plus, as a side note, then we don't even have e.g. IOMMUF_executable. Jan
