* [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled
@ 2021-06-17 17:38 Julien Grall
2021-06-21 9:36 ` Luca Fancellu
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Julien Grall @ 2021-06-17 17:38 UTC (permalink / raw)
To: xen-devel
Cc: raphning, doebel, Julien GralL, Ian Jackson, Wei Liu,
Juergen Gross, Julien Grall
From: Julien GralL <jgrall@amazon.com>
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@amazon.com>
----
This is currently based on top of:
https://lore.kernel.org/xen-devel/20210616144324.31652-1-julien@xen.org
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;
+ goto out;
+ } else
+ assert(lu_status->conn == conn);
if (!lu_check_lu_allowed()) {
if (now < lu_status->started_at + lu_status->timeout)
@@ -747,7 +758,7 @@ static const char *lu_start(const void *ctx, struct connection *conn,
lu_status->timeout = to;
lu_status->started_at = time(NULL);
- errno = delay_request(conn, conn->in, do_lu_start, NULL, false);
+ errno = delay_request(conn, conn->in, do_lu_start, conn, false);
return NULL;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled
2021-06-17 17:38 [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled Julien Grall
@ 2021-06-21 9:36 ` Luca Fancellu
2021-06-22 8:46 ` Juergen Gross
2021-06-22 9:23 ` Juergen Gross
2 siblings, 0 replies; 10+ messages in thread
From: Luca Fancellu @ 2021-06-21 9:36 UTC (permalink / raw)
To: Julien Grall
Cc: xen-devel, raphning, doebel, Julien GralL, Ian Jackson, Wei Liu,
Juergen Gross
> On 17 Jun 2021, at 18:38, Julien Grall <julien@xen.org> wrote:
>
> From: Julien GralL <jgrall@amazon.com>
>
> 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@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
>
> ----
>
> This is currently based on top of:
>
> https://lore.kernel.org/xen-devel/20210616144324.31652-1-julien@xen.org
>
> 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;
> + goto out;
> + } else
> + assert(lu_status->conn == conn);
>
> if (!lu_check_lu_allowed()) {
> if (now < lu_status->started_at + lu_status->timeout)
> @@ -747,7 +758,7 @@ static const char *lu_start(const void *ctx, struct connection *conn,
> lu_status->timeout = to;
> lu_status->started_at = time(NULL);
>
> - errno = delay_request(conn, conn->in, do_lu_start, NULL, false);
> + errno = delay_request(conn, conn->in, do_lu_start, conn, false);
>
> return NULL;
> }
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled
2021-06-17 17:38 [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled Julien Grall
2021-06-21 9:36 ` Luca Fancellu
@ 2021-06-22 8:46 ` Juergen Gross
2021-06-22 8:53 ` Julien Grall
2021-06-22 9:23 ` Juergen Gross
2 siblings, 1 reply; 10+ messages in thread
From: Juergen Gross @ 2021-06-22 8:46 UTC (permalink / raw)
To: Julien Grall, xen-devel
Cc: raphning, doebel, Julien GralL, Ian Jackson, Wei Liu
[-- Attachment #1.1.1: Type: text/plain, Size: 650 bytes --]
On 17.06.21 19:38, Julien Grall wrote:
> From: Julien GralL <jgrall@amazon.com>
>
> 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.
Umm, you introduced that bug in "[PATCH 03/10] tools/xenstore: Don't
assume conn->in points to the LU request".
So I think you should fold your fix into that series.
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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled
2021-06-22 8:46 ` Juergen Gross
@ 2021-06-22 8:53 ` Julien Grall
2021-06-22 9:13 ` Juergen Gross
0 siblings, 1 reply; 10+ messages in thread
From: Julien Grall @ 2021-06-22 8:53 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: raphning, doebel, Julien GralL, Ian Jackson, Wei Liu
Hi Juergen,
On 22/06/2021 10:46, Juergen Gross wrote:
> On 17.06.21 19:38, Julien Grall wrote:
>> From: Julien GralL <jgrall@amazon.com>
>>
>> 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.
>
> Umm, you introduced that bug in "[PATCH 03/10] tools/xenstore: Don't
> assume conn->in points to the LU request".
No. I did reproduced this one without my series. If there are in-flight
transaction this will crash in lu_check_lu_allowed() otherwise, it will
crash when calling lu_dump_state().
The easiest way to reproduce is requesting live-update when there are
long transactions and from a different connection (still in dom0)
requesting to abort the connection.
In this case, lu_abort() will free lu_status and the destructor will set
it to NULL. But the actual request is still in the delayed request queue
for the other connection.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled
2021-06-22 8:53 ` Julien Grall
@ 2021-06-22 9:13 ` Juergen Gross
0 siblings, 0 replies; 10+ messages in thread
From: Juergen Gross @ 2021-06-22 9:13 UTC (permalink / raw)
To: Julien Grall, xen-devel
Cc: raphning, doebel, Julien GralL, Ian Jackson, Wei Liu
[-- Attachment #1.1.1: Type: text/plain, Size: 993 bytes --]
On 22.06.21 10:53, Julien Grall wrote:
> Hi Juergen,
>
> On 22/06/2021 10:46, Juergen Gross wrote:
>> On 17.06.21 19:38, Julien Grall wrote:
>>> From: Julien GralL <jgrall@amazon.com>
>>>
>>> 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.
>>
>> Umm, you introduced that bug in "[PATCH 03/10] tools/xenstore: Don't
>> assume conn->in points to the LU request".
>
> No. I did reproduced this one without my series. If there are in-flight
> transaction this will crash in lu_check_lu_allowed() otherwise, it will
> crash when calling lu_dump_state().
Oh, right, I missed the indirection via delay_request().
Sorry.
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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled
2021-06-17 17:38 [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled Julien Grall
2021-06-21 9:36 ` Luca Fancellu
2021-06-22 8:46 ` Juergen Gross
@ 2021-06-22 9:23 ` Juergen Gross
2021-06-24 8:12 ` Julien Grall
2 siblings, 1 reply; 10+ messages in thread
From: Juergen Gross @ 2021-06-22 9:23 UTC (permalink / raw)
To: Julien Grall, xen-devel
Cc: raphning, doebel, Julien GralL, Ian Jackson, Wei Liu
[-- Attachment #1.1.1: Type: text/plain, Size: 1843 bytes --]
On 17.06.21 19:38, Julien Grall wrote:
> From: Julien GralL <jgrall@amazon.com>
>
> 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@amazon.com>
>
> ----
>
> This is currently based on top of:
>
> https://lore.kernel.org/xen-devel/20210616144324.31652-1-julien@xen.org
>
> 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.
Other than that:
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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled
2021-06-22 9:23 ` Juergen Gross
@ 2021-06-24 8:12 ` Julien Grall
2021-06-24 8:17 ` Juergen Gross
0 siblings, 1 reply; 10+ messages in thread
From: Julien Grall @ 2021-06-24 8:12 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: raphning, doebel, Julien GralL, Ian Jackson, Wei Liu
Hi Juergen,
On 22/06/2021 11:23, Juergen Gross wrote:
> On 17.06.21 19:38, Julien Grall wrote:
>> From: Julien GralL <jgrall@amazon.com>
>>
>> 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@amazon.com>
>>
>> ----
>>
>> This is currently based on top of:
>>
>> https://lore.kernel.org/xen-devel/20210616144324.31652-1-julien@xen.org
>>
>> 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@suse.com>
Thank you!
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled
2021-06-24 8:12 ` Julien Grall
@ 2021-06-24 8:17 ` Juergen Gross
2021-06-24 8:18 ` Julien Grall
0 siblings, 1 reply; 10+ messages in thread
From: Juergen Gross @ 2021-06-24 8:17 UTC (permalink / raw)
To: Julien Grall, xen-devel
Cc: raphning, doebel, Julien GralL, Ian Jackson, Wei Liu
[-- Attachment #1.1.1: Type: text/plain, Size: 2450 bytes --]
On 24.06.21 10:12, Julien Grall wrote:
> Hi Juergen,
>
> On 22/06/2021 11:23, Juergen Gross wrote:
>> On 17.06.21 19:38, Julien Grall wrote:
>>> From: Julien GralL <jgrall@amazon.com>
>>>
>>> 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@amazon.com>
>>>
>>> ----
>>>
>>> This is currently based on top of:
>>>
>>> https://lore.kernel.org/xen-devel/20210616144324.31652-1-julien@xen.org
>>>
>>> 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.
I guess you are planning to do the commit?
If yes, there is no need for resending the patch.
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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled
2021-06-24 8:17 ` Juergen Gross
@ 2021-06-24 8:18 ` Julien Grall
2021-06-24 11:17 ` Julien Grall
0 siblings, 1 reply; 10+ messages in thread
From: Julien Grall @ 2021-06-24 8:18 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: raphning, doebel, Julien GralL, Ian Jackson, Wei Liu
Hi Juergen,
On 24/06/2021 10:17, Juergen Gross wrote:
> On 24.06.21 10:12, Julien Grall wrote:
>> Hi Juergen,
>>
>> On 22/06/2021 11:23, Juergen Gross wrote:
>>> On 17.06.21 19:38, Julien Grall wrote:
>>>> From: Julien GralL <jgrall@amazon.com>
>>>>
>>>> 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@amazon.com>
>>>>
>>>> ----
>>>>
>>>> This is currently based on top of:
>>>>
>>>> https://lore.kernel.org/xen-devel/20210616144324.31652-1-julien@xen.org
>>>>
>>>> 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.
>
> I guess you are planning to do the commit?
That's my plan.
>
> If yes, there is no need for resending the patch.
Thanks!
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled
2021-06-24 8:18 ` Julien Grall
@ 2021-06-24 11:17 ` Julien Grall
0 siblings, 0 replies; 10+ messages in thread
From: Julien Grall @ 2021-06-24 11:17 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: raphning, doebel, Julien GralL, Ian Jackson, Wei Liu
On 24/06/2021 10:18, Julien Grall wrote:
> Hi Juergen,
>
> On 24/06/2021 10:17, Juergen Gross wrote:
>> On 24.06.21 10:12, Julien Grall wrote:
>>> Hi Juergen,
>>>
>>> On 22/06/2021 11:23, Juergen Gross wrote:
>>>> On 17.06.21 19:38, Julien Grall wrote:
>>>>> From: Julien GralL <jgrall@amazon.com>
>>>>>
>>>>> 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@amazon.com>
>>>>>
>>>>> ----
>>>>>
>>>>> This is currently based on top of:
>>>>>
>>>>> https://lore.kernel.org/xen-devel/20210616144324.31652-1-julien@xen.org
>>>>>
>>>>>
>>>>> 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.
>>
>> I guess you are planning to do the commit?
>
> That's my plan.
Committed.
>
>>
>> If yes, there is no need for resending the patch.
>
> Thanks!
>
> Cheers,
>
--
Julien Grall
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-06-24 11:17 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-17 17:38 [PATCH] tools/xenstored: Don't crash xenstored when Live-Update is cancelled Julien Grall
2021-06-21 9:36 ` Luca Fancellu
2021-06-22 8:46 ` Juergen Gross
2021-06-22 8:53 ` Julien Grall
2021-06-22 9:13 ` Juergen Gross
2021-06-22 9:23 ` Juergen Gross
2021-06-24 8:12 ` Julien Grall
2021-06-24 8:17 ` Juergen Gross
2021-06-24 8:18 ` Julien Grall
2021-06-24 11:17 ` Julien Grall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).