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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.10.2026 14:58, Roger Pau Monné wrote:
> 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:

Yes, this is fine with me:
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan

> --- 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.