Re: [Xen-devel] [PATCH v2] xen/sched: don't let XEN_RUNSTATE_UPDATE leak into vcpu_runstate_get()

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 9/25/19 5:29 AM, Juergen Gross wrote:
> vcpu_runstate_get() should never return a state entry time with
> XEN_RUNSTATE_UPDATE set. To avoid this let update_runstate_area()
> operate on a local runstate copy.
>
> This problem was introduced with commit 2529c850ea48f036 ("add update
> indicator to vcpu_runstate_info").
>
> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
> V2: add handling on ARM, too (Jan Beulich)
> ---
>   xen/arch/arm/domain.c | 13 ++++++++-----
>   xen/arch/x86/domain.c | 17 ++++++++++-------
>   2 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index ae13e47e86..d681ff5c6e 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -280,28 +280,31 @@ static void ctxt_switch_to(struct vcpu *n)
>   static void update_runstate_area(struct vcpu *v)
>   {
>       void __user *guest_handle = NULL;
> +    struct vcpu_runstate_info runstate;
>   
>       if ( guest_handle_is_null(runstate_guest(v)) )
>           return;
>   
> +    memcpy(&runstate, &v->runstate, sizeof(runstate));
I am not really happy with this solution. AFAICT, you only copy the full 
structure here just for the benefits of updating state_entry_time.
I saw you discuss about it with Jan, so it would be nice to log at least 
in the commit message the reason why this is done like that.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel