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

Re: [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled



Hi Juergen,

On 22/06/2021 11:23, Juergen Gross wrote:
On 17.06.21 19:38, Julien Grall wrote:
From: Julien GralL <jgrall@xxxxxxxxxx>

As Live-Update is asynchronous, it is possible to receive a request to
cancel it (either on the same connection or from a different one).

Currently, this will crash xenstored because do_lu_start() assumes
lu_status will be valid. This is not the case when Live-Update has been
cancelled. This will result to dereference a NULL pointer and
crash Xenstored.

Rework do_lu_start() to check if lu_status is NULL and return an
error in this case.

Fixes: af216a99fb ("tools/xenstore: add the basic framework for doing the live update")
Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>

----

This is currently based on top of:

https://lore.kernel.org/xen-devel/20210616144324.31652-1-julien@xxxxxxx

This can be re-ordered if necessary.
---
  tools/xenstore/xenstored_control.c | 15 +++++++++++++--
  1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index a045f102a420..37a3d39f20b5 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -696,7 +696,18 @@ static bool do_lu_start(struct delayed_request *req)
      time_t now = time(NULL);
      const char *ret;
      struct buffered_data *saved_in;
-    struct connection *conn = lu_status->conn;
+    struct connection *conn = req->data;
+
+    /*
+     * Cancellation may have been requested asynchronously. In this
+     * case, lu_status will be NULL.
+     */
+    if (!lu_status) {
+        ret = "Cancellation was requested";
+        conn = req->data;

This will set conn to the same value it already has.

Ah yes. I will drop this line.

Also, I took the opportunity to replace

} else
  assert(...)

with just

assert(...)

This should improve a bit the readability. Let me know if you want me to resend the patch for that.



Other than that:

Reviewed-by: Juergen Gross <jgross@xxxxxxxx>

Thank you!

Cheers,

--
Julien Grall



 


Rackspace

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