Re: [Xen-devel] [PATCH v6 07/12] livepatch: Add per-function applied/reverted state tracking marker

["Wieczorkiewicz, Pawel" <wipawel@xxxxxxxxx>]

On 5. Jan 2020, at 12:36, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:

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