Mailing List Archive

commit 5564323f643715f9d364df88e0eb9c7d6fd2c22b
Author: Roger Pau Monné <roger.pau@citrix.com>
AuthorDate: Tue Mar 5 11:59:43 2024 +0100
Commit: Jan Beulich <jbeulich@suse.com>
CommitDate: Tue Mar 5 11:59:43 2024 +0100

xen/livepatch: fix norevert test attempt to open-code revert

The purpose of the norevert test is to install a dummy handler that replaces
the internal Xen revert code, and then perform the revert in the post-revert
hook. For that purpose the usage of the previous common_livepatch_revert() is
not enough, as that just reverts specific functions, but not the whole state of
the payload.

Remove both common_livepatch_{apply,revert}() and instead expose
revert_payload{,_tail}() in order to perform the patch revert from the
post-revert hook.

Fixes: 6047104c3ccc ('livepatch: Add per-function applied/reverted state tracking marker')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
master commit: cdae267ce10d04d71d1687b5701ff2911a96b6dc
master date: 2024-02-28 16:57:25 +0000
---
xen/common/livepatch.c | 41 +++++++++++++++++++++++---
xen/include/xen/livepatch.h | 32 +++-----------------
xen/test/livepatch/xen_action_hooks_norevert.c | 22 ++++----------
3 files changed, 46 insertions(+), 49 deletions(-)

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index a129ab9973..a5068a2217 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -1310,7 +1310,22 @@ static int apply_payload(struct payload *data)
ASSERT(!local_irq_is_enabled());

for ( i = 0; i < data->nfuncs; i++ )
- common_livepatch_apply(&data->funcs[i], &data->fstate[i]);
+ {
+ const struct livepatch_func *func = &data->funcs[i];
+ struct livepatch_fstate *state = &data->fstate[i];
+
+ /* If the action has been already executed on this function, do nothing. */
+ if ( state->applied == LIVEPATCH_FUNC_APPLIED )
+ {
+ printk(XENLOG_WARNING LIVEPATCH
+ "%s: %s has been already applied before\n",
+ __func__, func->name);
+ continue;
+ }
+
+ arch_livepatch_apply(func, state);
+ state->applied = LIVEPATCH_FUNC_APPLIED;
+ }

arch_livepatch_revive();

@@ -1326,7 +1341,7 @@ static inline void apply_payload_tail(struct payload *data)
data->state = LIVEPATCH_STATE_APPLIED;
}

-static int revert_payload(struct payload *data)
+int revert_payload(struct payload *data)
{
unsigned int i;
int rc;
@@ -1341,7 +1356,25 @@ static int revert_payload(struct payload *data)
}

for ( i = 0; i < data->nfuncs; i++ )
- common_livepatch_revert(&data->funcs[i], &data->fstate[i]);
+ {
+ const struct livepatch_func *func = &data->funcs[i];
+ struct livepatch_fstate *state = &data->fstate[i];
+
+ /*
+ * If the apply action hasn't been executed on this function, do
+ * nothing.
+ */
+ if ( !func->old_addr || state->applied == LIVEPATCH_FUNC_NOT_APPLIED )
+ {
+ printk(XENLOG_WARNING LIVEPATCH
+ "%s: %s has not been applied before\n",
+ __func__, func->name);
+ continue;
+ }
+
+ arch_livepatch_revert(func, state);
+ state->applied = LIVEPATCH_FUNC_NOT_APPLIED;
+ }

/*
* Since we are running with IRQs disabled and the hooks may call common
@@ -1359,7 +1392,7 @@ static int revert_payload(struct payload *data)
return 0;
}

-static inline void revert_payload_tail(struct payload *data)
+void revert_payload_tail(struct payload *data)
{
list_del(&data->applied_list);

diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 537d3d58b6..c9ee58fd37 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -136,35 +136,11 @@ void arch_livepatch_post_action(void);
void arch_livepatch_mask(void);
void arch_livepatch_unmask(void);

-static inline void common_livepatch_apply(const struct livepatch_func *func,
- struct livepatch_fstate *state)
-{
- /* If the action has been already executed on this function, do nothing. */
- if ( state->applied == LIVEPATCH_FUNC_APPLIED )
- {
- printk(XENLOG_WARNING LIVEPATCH "%s: %s has been already applied before\n",
- __func__, func->name);
- return;
- }
-
- arch_livepatch_apply(func, state);
- state->applied = LIVEPATCH_FUNC_APPLIED;
-}
+/* Only for testing purposes. */
+struct payload;
+int revert_payload(struct payload *data);
+void revert_payload_tail(struct payload *data);

-static inline void common_livepatch_revert(const struct livepatch_func *func,
- struct livepatch_fstate *state)
-{
- /* If the apply action hasn't been executed on this function, do nothing. */
- if ( !func->old_addr || state->applied == LIVEPATCH_FUNC_NOT_APPLIED )
- {
- printk(XENLOG_WARNING LIVEPATCH "%s: %s has not been applied before\n",
- __func__, func->name);
- return;
- }
-
- arch_livepatch_revert(func, state);
- state->applied = LIVEPATCH_FUNC_NOT_APPLIED;
-}
#else

/*
diff --git a/xen/test/livepatch/xen_action_hooks_norevert.c b/xen/test/livepatch/xen_action_hooks_norevert.c
index c173855192..c5fbab1746 100644
--- a/xen/test/livepatch/xen_action_hooks_norevert.c
+++ b/xen/test/livepatch/xen_action_hooks_norevert.c
@@ -96,26 +96,14 @@ static int revert_hook(livepatch_payload_t *payload)

static void post_revert_hook(livepatch_payload_t *payload)
{
- int i;
+ unsigned long flags;

printk(KERN_DEBUG "%s: Hook starting.\n", __func__);

- for (i = 0; i < payload->nfuncs; i++)
- {
- const struct livepatch_func *func = &payload->funcs[i];
- struct livepatch_fstate *fstate = &payload->fstate[i];
-
- BUG_ON(revert_cnt != 1);
- BUG_ON(fstate->applied != LIVEPATCH_FUNC_APPLIED);
-
- /* Outside of quiesce zone: MAY TRIGGER HOST CRASH/UNDEFINED BEHAVIOR */
- arch_livepatch_quiesce();
- common_livepatch_revert(payload);
- arch_livepatch_revive();
- BUG_ON(fstate->applied == LIVEPATCH_FUNC_APPLIED);
-
- printk(KERN_DEBUG "%s: post reverted: %s\n", __func__, func->name);
- }
+ local_irq_save(flags);
+ BUG_ON(revert_payload(payload));
+ revert_payload_tail(payload);
+ local_irq_restore(flags);

printk(KERN_DEBUG "%s: Hook done.\n", __func__);
}
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.17