[PATCH] tools/xenstored: Stash the correct request in lu_status->in

[PATCH] tools/xenstored: Stash the correct request in lu_status->in

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

When Live-Updating with some load, Xenstored may hit the assert
req->in == lu_status->in in do_lu_start().

This is happening because the request is stashed when Live-Update
begins. This happens in a different request (see call lu_begin()
when select the new binary) from the one performing Live-Update.

To avoid the problem, stash the request in lu_start().

Fixes: 65f19ed62aa1 ("tools/xenstore: Don't assume conn->in points to the LU request")
Reported-by: Michael Kurth <mku@amazon.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>

----

This was sadly missed because the on my testing the 2 requests were
residing at the same place in memory.

This was reproduced by creating domain while Live-Updating. Without
the patch, Xenstored will crash.
---
 tools/xenstore/xenstored_control.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index a1b1bd5a718d..ff9863c17fa4 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -103,7 +103,6 @@ static const char *lu_begin(struct connection *conn)
 	if (!lu_status)
 		return "Allocation failure.";
 	lu_status->conn = conn;
-	lu_status->in = conn->in;
 	talloc_set_destructor(lu_status, lu_destroy);
 
 	return NULL;
@@ -757,6 +756,7 @@ static const char *lu_start(const void *ctx, struct connection *conn,
 	lu_status->force = force;
 	lu_status->timeout = to;
 	lu_status->started_at = time(NULL);
+	lu_status->in = conn->in;
 
 	errno = delay_request(conn, conn->in, do_lu_start, conn, false);
 
-- 
2.17.1

Re: [PATCH] tools/xenstored: Stash the correct request in lu_status->in

[Luca Fancellu]

> On 1 Jul 2021, at 15:03, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> When Live-Updating with some load, Xenstored may hit the assert
> req->in == lu_status->in in do_lu_start().
> 
> This is happening because the request is stashed when Live-Update
> begins. This happens in a different request (see call lu_begin()
> when select the new binary) from the one performing Live-Update.
> 
> To avoid the problem, stash the request in lu_start().
> 
> Fixes: 65f19ed62aa1 ("tools/xenstore: Don't assume conn->in points to the LU request")
> Reported-by: Michael Kurth <mku@amazon.com>
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: luca.fancellu@arm.com

> 
> ----
> 
> This was sadly missed because the on my testing the 2 requests were
> residing at the same place in memory.
> 
> This was reproduced by creating domain while Live-Updating. Without
> the patch, Xenstored will crash.
> ---
> tools/xenstore/xenstored_control.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
> index a1b1bd5a718d..ff9863c17fa4 100644
> --- a/tools/xenstore/xenstored_control.c
> +++ b/tools/xenstore/xenstored_control.c
> @@ -103,7 +103,6 @@ static const char *lu_begin(struct connection *conn)
> 	if (!lu_status)
> 		return "Allocation failure.";
> 	lu_status->conn = conn;
> -	lu_status->in = conn->in;
> 	talloc_set_destructor(lu_status, lu_destroy);
> 
> 	return NULL;
> @@ -757,6 +756,7 @@ static const char *lu_start(const void *ctx, struct connection *conn,
> 	lu_status->force = force;
> 	lu_status->timeout = to;
> 	lu_status->started_at = time(NULL);
> +	lu_status->in = conn->in;
> 
> 	errno = delay_request(conn, conn->in, do_lu_start, conn, false);
> 
> -- 
> 2.17.1
> 
> 

Re: [PATCH] tools/xenstored: Stash the correct request in lu_status->in

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 723 bytes --]

On 01.07.21 16:03, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> When Live-Updating with some load, Xenstored may hit the assert
> req->in == lu_status->in in do_lu_start().
> 
> This is happening because the request is stashed when Live-Update
> begins. This happens in a different request (see call lu_begin()
> when select the new binary) from the one performing Live-Update.
> 
> To avoid the problem, stash the request in lu_start().
> 
> Fixes: 65f19ed62aa1 ("tools/xenstore: Don't assume conn->in points to the LU request")
> Reported-by: Michael Kurth <mku@amazon.com>
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] tools/xenstored: Stash the correct request in lu_status->in

[Julien Grall]

Hi Juergen,

On 05/07/2021 13:49, Juergen Gross wrote:
> On 01.07.21 16:03, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> When Live-Updating with some load, Xenstored may hit the assert
>> req->in == lu_status->in in do_lu_start().
>>
>> This is happening because the request is stashed when Live-Update
>> begins. This happens in a different request (see call lu_begin()
>> when select the new binary) from the one performing Live-Update.
>>
>> To avoid the problem, stash the request in lu_start().
>>
>> Fixes: 65f19ed62aa1 ("tools/xenstore: Don't assume conn->in points to 
>> the LU request")
>> Reported-by: Michael Kurth <mku@amazon.com>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> Reviewed-by: Juergen Gross <jgross@suse.com>

Committed. Thanks!

Cheers,

-- 
Julien Grall