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

[Jason Andryuk <jason.andryuk@xxxxxxx>]

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