Re: [PATCH 14/23] xsm/dummy: Allow XS_PRIV to call get_hvm_param

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2025-03-17 11:19, Juergen Gross wrote:
> On 17.03.25 15:50, Jason Andryuk wrote:
> > On 2025-03-17 10:18, Jan Beulich wrote:
> > > On 06.03.2025 23:03, Jason Andryuk wrote:
> > > > This is useful for a combined hardware/xenstore domain that will run
> > > > init-dom0less and xenstored.  init-dom0less calls xc_hvm_param_get() to
> > > > retrieve the xenstore event channel and pfn to configure xenstore for a
> > > > guest.  With a hypervisor-allocated event channel and page, the
> > > > set_hvm_param is not needed, and the normal domid permissions will 
> > > > allow
> > > > xenstored to connect.
> > > >
> > > > Similarly, a hyperlaunch-ed xenstore stubdom needs to read a domain's
> > > > xenstore event channel out of hvm_param.
> > > >
> > > > This allows reading but not modifying the guest, so allow the 
> > > > permission.
> > > > Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> > > Since this is exposing the entire param space to Xenstore, what I'm 
> > > missing
> > > is a security discussion for existing as well as potential future 
> > > params.
> > > There could well be some that better wouldn't be available for 
> > > Xenstrore to
> > > fetch.
> > I can't speak for future parameters, but existing HVM_PARAMs didn't 
> > seem sensitive to me.  The safest choice is to just pass the index to 
> > xsm_hvm_param() and allow just HVM_PARAM_STORE_EVTCHN (and 
> > HVM_PARAM_STORE_PFN) for the xenstore domain.
> > This works for ARM and x86 HVM/PVH.  PV doesn't have a way to 
> > determine a domain's event channel port, FWICT.
> For what are you needing HVM_PARAM_STORE_PFN? The GNTTAB_RESERVED_XENSTORE
> grant id should be enough to map the guest's Xenstore page?
Indeed, HVM_PARAM_STORE_PFN is not needed and I expect only using 
GNTTAB_RESERVED_XENSTORE to map the grant.  I just listed it as a 
compliment to the event channel.
> And with that I'd rather suggest to expand struct xenstore_domain_interface
> with the event channel port and let the component doing the seeding of the
> grant table write the port into the struct.
>
> This would enable Xenstore to just map the guest's Xenstore page and read
> the event channel port from it. No additional hypercall permission needed.
> And this would even work with PV domains. And as the Xenstore page is 
> zeroed
> initially, any event channel port != 0 could be regarded to be valid (a 
> guest
> choosing to write a bogus value there would just shoot itself in the 
> foot by
> harming its own Xenstore connection).
Yes, that is a good idea.  I had also considered defining a reserved 
event channel for xenstore, but wasn't sure how that would be received. 
I like this better.
Thanks,
Jason