[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v6 07/12] livepatch: Add per-function applied/reverted state tracking marker
- To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- From: "Wieczorkiewicz, Pawel" <wipawel@xxxxxxxxx>
- Date: Mon, 6 Jan 2020 12:22:06 +0000
- Accept-language: en-US
- Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Wei Liu <wl@xxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, "Pohlack, Martin" <mpohlack@xxxxxxxxx>, "Wieczorkiewicz, Pawel" <wipawel@xxxxxxxxx>, "Manthey, Norbert" <nmanthey@xxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Mon, 06 Jan 2020 12:22:41 +0000
- Ironport-sdr: kVCF/wDxNvuwzTWj2JM8cD8BmZX8R2zVAp4XkmbKLFmoC+KfDk5gfORb737RGdrdSmj5Aqi15S uAMlgLZLQH9w==
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Thread-index: AQHVpEF02rAa6UooNU61iH0I9SNjr6fcMEMAgAGfCQA=
- Thread-topic: [PATCH v6 07/12] livepatch: Add per-function applied/reverted state tracking marker
On 26/11/2019 10:07, Pawel Wieczorkiewicz wrote:
@@ -1274,6 +1297,9 @@ static void livepatch_do_action(void)
else
<snip>
break;
@@ -1309,6 +1338,9 @@ static void livepatch_do_action(void)
else
other->rc = revert_payload(other);
+ if ( !was_action_consistent(other, rc ? LIVEPATCH_FUNC_APPLIED : LIVEPATCH_FUNC_NOT_APPLIED) )
+ panic("livepatch: partially reverted payload '%s'!\n", other->name);
+
if ( other->rc == 0 )
revert_payload_tail(other);
Coverity highlights that this contains dead code.
The LIVEPATCH_ACTION_REPLACE case, unlike all others, uses other->rc,
which means the rc ? : check will always pass LIVEPATCH_FUNC_APPLIED
into was_action_consistent(), due to the rc = 0 at the head of the case
block.
Yes, this has to be other->rc instead of rc. Thanks!
If this were the only problem, switching rc to other->rc might be ok,
but there look to be other confusions in the surrounding code. Would
you mind looking over the whole block of code for correct error handling?
What are the confusions in the code? Could you be more specific and point me to them?
I have just checked the LIVEPATCH_ACTION_REPLACE case block again.
It looks correct to me. That is, it preserves the original logic of error handling there.
I just added the extensions. But, the flow for rc and other->rc should be the same
and correct (modulo the was_action_consistent() bug).
For any resulting patch, the Coverity ID is 1457467
~Andrew
else
@@ -1329,6 +1361,9 @@ static void livepatch_do_action(void)
else
rc = apply_payload(data);
+ if ( !was_action_consistent(data, rc ? LIVEPATCH_FUNC_NOT_APPLIED : LIVEPATCH_FUNC_APPLIED) )
+ panic("livepatch: partially applied payload '%s'!\n", data->name);
+
if ( rc == 0 )
apply_payload_tail(data);
}
Best Regards,
Pawel Wieczorkiewicz
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
|
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|