[PATCH v2 06/17] xen/xsm: Expand XSM_XS_PRIV for untargetable domains

Jason Andryuk posted 17 patches 3 months, 2 weeks ago
[PATCH v2 06/17] xen/xsm: Expand XSM_XS_PRIV for untargetable domains
Posted by Jason Andryuk 3 months, 2 weeks ago
Untargetable domains are currently hidden from the control domain -
e.g. xl list will not show them.  getdomaininfo fails on the
!is_hypercall_target() check in XSM_TARGET.  Add control domain to the
XSM_XS_PRIV check so it can pass.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
 xen/include/xsm/dummy.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index f2205575ed..4d7b1d61eb 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -87,7 +87,8 @@ static always_inline int xsm_default_action(
         fallthrough;
     case XSM_XS_PRIV:
         if ( action == XSM_XS_PRIV &&
-             evaluate_nospec(is_xenstore_domain(src)) )
+             (evaluate_nospec(is_xenstore_domain(src)) ||
+              is_control_domain(src)) )
             return 0;
         fallthrough;
     case XSM_DM_PRIV:
-- 
2.50.0
Re: [PATCH v2 06/17] xen/xsm: Expand XSM_XS_PRIV for untargetable domains
Posted by Jan Beulich 3 months ago
On 16.07.2025 23:14, Jason Andryuk wrote:
> Untargetable domains are currently hidden from the control domain -
> e.g. xl list will not show them.  getdomaininfo fails on the
> !is_hypercall_target() check in XSM_TARGET.  Add control domain to the
> XSM_XS_PRIV check so it can pass.

This feels like a pretty gross abuse of XS_PRIV. It once again supports
my take that the level of granularity you want will require Flask. Or of
course you could also come up with a new access control mechanism, much
lighter-weight than Flask, but still not dummy nor SILO.

As per my reply to an earlier patch - whether it is okay to prevent e.g.
"xl list" to see all domains is questionable as well. I'm not seeing
"interference" there.

> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -87,7 +87,8 @@ static always_inline int xsm_default_action(
>          fallthrough;
>      case XSM_XS_PRIV:
>          if ( action == XSM_XS_PRIV &&
> -             evaluate_nospec(is_xenstore_domain(src)) )
> +             (evaluate_nospec(is_xenstore_domain(src)) ||
> +              is_control_domain(src)) )

Like in patch 2 (in is_priv_domain()), I think this wants collapsing the
two evaluate_nospec() into a single one.

Jan
Re: [PATCH v2 06/17] xen/xsm: Expand XSM_XS_PRIV for untargetable domains
Posted by Jason Andryuk 3 months ago
On 2025-07-30 11:19, Jan Beulich wrote:
> On 16.07.2025 23:14, Jason Andryuk wrote:
>> Untargetable domains are currently hidden from the control domain -
>> e.g. xl list will not show them.  getdomaininfo fails on the
>> !is_hypercall_target() check in XSM_TARGET.  Add control domain to the
>> XSM_XS_PRIV check so it can pass.
> 
> This feels like a pretty gross abuse of XS_PRIV. It once again supports
> my take that the level of granularity you want will require Flask. Or of
> course you could also come up with a new access control mechanism, much
> lighter-weight than Flask, but still not dummy nor SILO.

I considered a new XSM module, but it just seemed so close to the dummy 
policy.  From my perspective the board permissions of 
xsm_default_action() already implement the coarse permissions we need.

SILO works well because it builds on that and overrides a few hooks.  A 
new XSM module would need to re-implement ~every hook since 
xsm_default_action() needs to change.  So expanding xsm_default_action() 
seemed like a better approach to try.

> As per my reply to an earlier patch - whether it is okay to prevent e.g.
> "xl list" to see all domains is questionable as well. I'm not seeing
> "interference" there.

Yes, hiding domains from xl list seemed like it was going too far.  A 
system administrator should be able to see everything running on the 
system.  That's why this patch allows it.

>> --- a/xen/include/xsm/dummy.h
>> +++ b/xen/include/xsm/dummy.h
>> @@ -87,7 +87,8 @@ static always_inline int xsm_default_action(
>>           fallthrough;
>>       case XSM_XS_PRIV:
>>           if ( action == XSM_XS_PRIV &&
>> -             evaluate_nospec(is_xenstore_domain(src)) )
>> +             (evaluate_nospec(is_xenstore_domain(src)) ||
>> +              is_control_domain(src)) )
> 
> Like in patch 2 (in is_priv_domain()), I think this wants collapsing the
> two evaluate_nospec() into a single one.

Ok, thanks.

Regards,
Jason
Re: [PATCH v2 06/17] xen/xsm: Expand XSM_XS_PRIV for untargetable domains
Posted by Stefano Stabellini 3 months, 2 weeks ago
On Wed, 16 Jul 2025, Jason Andryuk wrote:
> Untargetable domains are currently hidden from the control domain -
> e.g. xl list will not show them.  getdomaininfo fails on the
> !is_hypercall_target() check in XSM_TARGET.  Add control domain to the
> XSM_XS_PRIV check so it can pass.
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>

XSM_XS_PRIV is only used in XEN_DOMCTL_get_domain_state,
XEN_DOMCTL_getdomaininfo and XEN_SYSCTL_getdomaininfolist, so it makes
sense to me

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/include/xsm/dummy.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index f2205575ed..4d7b1d61eb 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -87,7 +87,8 @@ static always_inline int xsm_default_action(
>          fallthrough;
>      case XSM_XS_PRIV:
>          if ( action == XSM_XS_PRIV &&
> -             evaluate_nospec(is_xenstore_domain(src)) )
> +             (evaluate_nospec(is_xenstore_domain(src)) ||
> +              is_control_domain(src)) )
>              return 0;
>          fallthrough;
>      case XSM_DM_PRIV:
> -- 
> 2.50.0
> 
>