On 3/31/22 08:36, Roger Pau Monné wrote:
> On Wed, Mar 30, 2022 at 07:05:48PM -0400, Daniel P. Smith wrote:
>> There are now instances where internal hypervisor logic needs to make
>> resource
>> allocation calls that are protected by XSM checks. The internal hypervisor
>> logic
>> is represented a number of system domains which by designed are represented
>> by
>> non-privileged struct domain instances. To enable these logic blocks to
>> function correctly but in a controlled manner, this commit introduces a pair
>> of privilege escalation and demotion functions that will make a system domain
>> privileged and then remove that privilege.
>>
>> Signed-off-by: Daniel P. Smith <dpsm...@apertussolutions.com>
>> ---
>> xen/include/xsm/xsm.h | 22 ++++++++++++++++++++++
>
> I'm not sure this needs to be in xsm code, AFAICT it could live in a
> more generic file.
>From my perspective this is access control logic, thus why I advocate
for it to be under XSM. A personal goal is to try to get all security,
i.e. access control, centralized to the extent that it doing so does not
make the code base unnecessarily complicated.
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
>> index e22d6160b5..157e57151e 100644
>> --- a/xen/include/xsm/xsm.h
>> +++ b/xen/include/xsm/xsm.h
>> @@ -189,6 +189,28 @@ struct xsm_operations {
>> #endif
>> };
>>
>> +static always_inline int xsm_elevate_priv(struct domain *d)
>
> I don't think it needs to be always_inline, using just inline would be
> fine IMO.
>
> Also this needs to be __init.
AIUI always_inline is likely the best way to preserve the speculation
safety brought in by the call to is_system_domain().
>> +{
>> + if ( is_system_domain(d) )
>> + {
>> + d->is_privileged = true;
>> + return 0;
>> + }
>> +
>
> I would add an ASSERT_UNREACHABLE(); here, I don't think we have any
> use case for trying to elevate the privileges of a non-system domain.
Ack.
v/r
dps
