Re: [PATCH 17/24] XSM: make Argo hooks well-formed ones

[Jason Andryuk <jason.andryuk@amd.com>]

On 2026-08-13 08:09, Daniel P. Smith wrote:
> On 8/6/26 10:09 AM, Jason Andryuk wrote:
>> On 2026-08-06 03:16, Jan Beulich wrote:
>>> On 06.08.2026 01:02, Daniel P. Smith wrote:
>>>> On 7/28/26 9:22 AM, Jan Beulich wrote:
>>>>> @@ -2307,7 +2308,7 @@ argo_init(struct domain *d)
>>>>>    {
>>>>>        struct argo_domain *argo;
>>>>> -    if ( !opt_argo || xsm_argo_enable(d) )
>>>>> +    if ( !opt_argo || xsm_argo_enable(XSM_HOOK, d) )
>>>>
>>>> This question came up on another thread, so thought I might point it 
>>>> out
>>>> that when FLASK is in use this can return a nubmer of error codes 
>>>> beyond
>>>> an access deny. While I know it's the existing behavior, but if the
>>>> error code is anything other than -EPERM, then it's not that the policy
>>>> denied the access but something cause a fault in the security 
>>>> server. In
>>>> that case the domain is still being allowed to construct with the
>>>> assumption that it was a policy deny. At a minimum should the error 
>>>> code
>>>> at least get reported, and perhaps it should be passed up to domain
>>>> construction to allowing it to make an informed decision on 
>>>> construction?
>>>
>>> Sounds plausible, but definitely wants doing in a separate patch.
>>
>> I think this is a mis-use of xsm_argo_enable().  As I wrote in [1], 
>> this isn't an access decision, but an ~optimization to skip 
>> initializing argo data structures when a domain is not allowed to use 
>> argo.
>>
> 
> I would have to respectfully disagree. The operation is to initialize 
> the domain for argo usage and the access check says do not allow 
> initialization if the domain does not have the privilege. This basic 
> defense in depth, do not initialize for some thing you should not have 
> access to, and thus not just relying on the later checks.

I was thinking of it as robustness.  If you always initialize argo, then 
you don't have to check ->argo for NULL for each domain.  Though, if you 
want to selectively allow argo for individual domains, you have to check 
something anyway.

>> With Flask, this prints an AVC denial during domain construction when 
>> the domain doesn't have argo enabled.  That is misleading as it isn't 
>> the domain's action causing the access.  In OpenXT, I wrote a patch to 
>> add a noaudit variant to hide the denial.  I didn't upstream it 
>> because I didn't really like it.
>>
> 
> The customer has ran OpenXT through the code evaluation models that they 
> have access to and your patch was flagged. Not for being technically 
> incorrect, but raised policy questions on whether is was desirable to 
> silence the event.
Again, xsm_argo_enable(d) is used for two purposes:
  - Access to the argo_op hypercall: current == d
  - This argo_init(d) call: current != d

Domain create always goes through argo_init().  current triggers the 
denial, but it is logged against d.  This is misleading as d did not 
perform any operation.

Regards,
Jason