Re: [PATCH v2] xen/vcpu: ensure no write when runstate handler is NULL

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

On Mon, Oct 26, 2026 at 02:06:48PM +0100, Jan Beulich wrote:
> On 23.10.2026 18:17, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/x86_64/domain.c
> > +++ b/xen/arch/x86/x86_64/domain.c
> > @@ -58,6 +58,11 @@ compat_vcpu_op(int cmd, unsigned int vcpuid, 
> > XEN_GUEST_HANDLE_PARAM(void) arg)
> >          rc = 0;
> >          guest_from_compat_handle(v->runstate_guest.compat, area.addr.h);
> >  
> > +        /* NULL handle is deregistering the address. */
> > +        if ( compat_handle_is_null(area.addr.h) )
> > +            break;
> > +
> > +        /* Update the newly-registered runstate area, best-effort. */
> >          if ( v == current )
> >          {
> >              XLAT_vcpu_runstate_info(&info, &v->runstate);
> 
> While likely unnecessary in this x86-only handling, ...
> 
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -2330,6 +2330,11 @@ long common_vcpu_op(int cmd, struct vcpu *v, 
> > XEN_GUEST_HANDLE_PARAM(void) arg)
> >          rc = 0;
> >          runstate_guest(v) = area.addr.h;
> >  
> > +        /* NULL handle is deregistering the address. */
> > +        if ( guest_handle_is_null(area.addr.h) )
> > +            break;
> > +
> > +        /* Update the newly-registered runstate area, best-effort. */
> >          if ( v == current )
> >          {
> >              __copy_to_guest(runstate_guest(v), &v->runstate, 1);
> 
> ... if we make null an official "de-register", then imo the earlier
> guest_handle_okay() also wants bypassing in the null case. An arch
> could - in principle - reject a null handle.

Yes, it could en theory be possible, albeit I think it's very unlikely
we gain anything similar to x86 PV elsewhere.  Would you be OK with:

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6b52713518da..b41be89cd0b9 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -2324,12 +2324,21 @@ long common_vcpu_op(int cmd, struct vcpu *v, 
XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&area, arg, 1) )
             break;

+        /* NULL handle is deregistering the address. */
+        if ( guest_handle_is_null(area.addr.h) )
+        {
+            runstate_guest(v) = area.addr.h;
+            rc = 0;
+            break;
+        }
+
         if ( !guest_handle_okay(area.addr.h, 1) )
             break;

         rc = 0;
         runstate_guest(v) = area.addr.h;

+        /* Update the newly-registered runstate area, best-effort. */
         if ( v == current )
         {
             __copy_to_guest(runstate_guest(v), &v->runstate, 1);

Thanks, Roger.