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

[Jürgen Groß <jgross@xxxxxxxx>]

On 26.09.19 16:27, Julien Grall wrote:
> 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.
Isn't the reference to commit 2529c850ea48f036 enough? The update
protocol is clearly described in that commit message.


Juergen

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