[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hi,

On 9/26/19 3:45 PM, Jürgen Groß wrote:
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.

I meant the reason to use the 'memcpy', which sounds like quite a waste, compare to only copy state_entry_time.

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.