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

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

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.
> 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.
> The issue is really the conditional initialization of d->argo.  It seems 
> like it would be simpler to always initialized argo for all domains. 
> xsm_argo_enable() would still prevent using it, but internal checking of 
> d->argo could be removed.
Defense in depth is about minimizing risk at cost of a degree of 
overhead. The dance will always be about how to balance the two.
> Regards,
> Jason
>
> [1] https://lore.kernel.org/xen-devel/758c8410- 
> a18e-45dc-8944-5913e5832397@xxxxxxxx/T/ 
> #m149e1571b819a0cf481eea558c6220a5a24d5285