Re: [PATCH v2 08/10] xsm: remove xsm_default_t from hook definitions

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 16/07/2021 08:23, Jan Beulich wrote:
> On 12.07.2021 22:32, Daniel P. Smith wrote:
>> The passing of an xsm_default_t at each of the xsm hook call sites
>> served different functions depending on whether XSM was enabled or not.
>> When XSM was not enabled it attempted to function as a link-time check
>> that declared default action at the call site matched the default
>> declared action for that hook in the dummy policy. When XSM was enabled,
>> it would just drop the  parameter.
>>
>> The removal of these values is two fold. They are a redundancy that
>> provides little context, especially when the value is XSM_OTHER.
> For XSM_OTHER I may agree, but in general I find the call-site uses
> helpful to know at least the rough level of intended restriction.
> E.g. ...
>
>> --- a/xen/arch/x86/cpu/mcheck/mce.c
>> +++ b/xen/arch/x86/cpu/mcheck/mce.c
>> @@ -1376,7 +1376,7 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
>>      struct xen_mc_msrinject *mc_msrinject;
>>      struct xen_mc_mceinject *mc_mceinject;
>>  
>> -    ret = xsm_do_mca(XSM_PRIV);
>> +    ret = xsm_do_mca();
> ... to now understand what this enforces (or not) I have to go to
> the actual implementation, even if I only want to know the trivial
> dummy incarnation of it. This effectively extends the "provides
> little context" from XSM_OTHER to all hooks.

The old scheme was even worse because it was only a plausible
approximation for the !XSM case while being actively wrong for SILO and
FLASK.

Furthermore, someone reading the code could be forgiven for thinking
that XSM_HOOK was something other than dead code.

~Andrew