Re: [PATCH 19/23] xsm/dummy: Allow sysctls to both hardware and control

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2025-03-17 10:36, Jan Beulich wrote:
> On 06.03.2025 23:03, Jason Andryuk wrote:
> > xl queries SYSCTL_physinfo for the physical cpus:
> > domU:~# xl list
> > libxl: error: libxl_utils.c:817:libxl_cpu_bitmap_alloc: failed to retrieve the 
> > maximum number of cpus
> > libxl: error: libxl_utils.c:817:libxl_cpu_bitmap_alloc: failed to retrieve the 
> > maximum number of cpus
> > libxl: error: libxl_utils.c:817:libxl_cpu_bitmap_alloc: failed to retrieve the 
> > maximum number of cpus
> > Name                    ID   Mem VCPUs        State   Time(s)
> > Domain-0                 0   800     1     r-----     130.0
> > dom0less-1               1   400     1     r-----     130.3
> > dom0less-2               2   800     1     r-----     130.3
> >
> > Hardware and control are both privileged.  Allow them both access to
> > sysctls so they have insight into the running system.  This is coarse
> > grained permissions for the dummy policy.
> In an earlier patch you alluded to the control domain being guarded against
> the hardware one. Shouldn't hwdom be limited operations retrieving info,
> but refused to make any changes to the system? Or maybe some kinds of
> changes are to be done by hwdom, but then shouldn't be possible to be made
> by the control domain?
As an example, with ACPI living in the hardware_domain, it would be the 
domain to issue XEN_SYSCTL_pm_op to upload cpufreq data.  But then 
control domain should be in charge of controlling how the system is 
running by setting cpufreq parameters.
> > --- a/xen/include/xsm/dummy.h
> > +++ b/xen/include/xsm/dummy.h
> > @@ -194,9 +194,10 @@ static XSM_INLINE int cf_check xsm_sysctl(XSM_DEFAULT_ARG 
> > int cmd)
> >       case XEN_SYSCTL_getdomaininfolist:
> >           return xsm_default_action(XSM_XS_PRIV, current->domain, NULL);
> >       case XEN_SYSCTL_readconsole:
> > -    case XEN_SYSCTL_physinfo:
> Didn't you add this line just a single patch ago?
Yes.  The previous patch was a minimal change.  This patch is a maximal 
change.  I thought this could be rejected and didn't want to merge the 
two.  Though XEN_SYSCTL_physinfo should be handled better even for the 
minimal change.
This patch may go too far, hardware domain does have legitimate use to 
some sysctls.  For a coarse, base policy I went with allowing more to 
hardware domain.  Hardware domain can't be untrusted, so I erred on the 
side of more permissions rather than fewer.
Regards,
Jason