@@ -174,10 +174,13 @@ extern void *const __initdata_cf_clobber_end[];
* The caller will set the "force" argument to true for the final
* invocation, such that no CALLs/JMPs to NULL pointers will be left
* around. See also the further comment below.
+ *
+ * Note the function cannot fail if system_state < SYS_STATE_active, it would
+ * panic instead. The return value is only meaningful for runtime usage.
*/
-static void init_or_livepatch _apply_alternatives(struct alt_instr *start,
- struct alt_instr *end,
- bool force)
+static int init_or_livepatch _apply_alternatives(struct alt_instr *start,
+ struct alt_instr *end,
+ bool force)
{
struct alt_instr *a, *base;
@@ -198,9 +201,17 @@ static void init_or_livepatch _apply_alternatives(struct alt_instr *start,
uint8_t buf[MAX_PATCH_LEN];
unsigned int total_len = a->orig_len + a->pad_len;
- BUG_ON(a->repl_len > total_len);
- BUG_ON(total_len > sizeof(buf));
- BUG_ON(a->cpuid >= NCAPINTS * 32);
+#define BUG_ON_BOOT(cond) \
+ if ( system_state < SYS_STATE_active ) \
+ BUG_ON(cond); \
+ else if ( cond ) \
+ return -EINVAL;
+
+ BUG_ON_BOOT(a->repl_len > total_len);
+ BUG_ON_BOOT(total_len > sizeof(buf));
+ BUG_ON_BOOT(a->cpuid >= NCAPINTS * 32);
+
+#undef BUG_ON_BOOT
/*
* Detect sequences of alt_instr's patching the same origin site, and
@@ -356,12 +367,14 @@ static void init_or_livepatch _apply_alternatives(struct alt_instr *start,
printk("altcall: Optimised away %u endbr64 instructions\n", clobbered);
}
+
+ return 0;
}
#ifdef CONFIG_LIVEPATCH
-void apply_alternatives(struct alt_instr *start, struct alt_instr *end)
+int apply_alternatives(struct alt_instr *start, struct alt_instr *end)
{
- _apply_alternatives(start, end, true);
+ return _apply_alternatives(start, end, true);
}
#endif
@@ -24,7 +24,8 @@ struct __packed alt_instr {
extern void add_nops(void *insns, unsigned int len);
/* Similar to alternative_instructions except it can be run with IRQs enabled. */
-extern void apply_alternatives(struct alt_instr *start, struct alt_instr *end);
+extern int __must_check apply_alternatives(struct alt_instr *start,
+ struct alt_instr *end);
extern void alternative_instructions(void);
extern void alternative_branches(void);
@@ -882,7 +882,15 @@ static int prepare_payload(struct payload *payload,
return -EINVAL;
}
}
- apply_alternatives(start, end);
+
+ rc = apply_alternatives(start, end);
+ if ( rc )
+ {
+ printk(XENLOG_ERR LIVEPATCH "%s applying alternatives failed: %d\n",
+ elf->name, rc);
+ return rc;
+ }
+
alt_done:;
#else
printk(XENLOG_ERR LIVEPATCH "%s: We don't support alternative patching\n",
@@ -125,6 +125,11 @@ $(obj)/xen_action_hooks_norevert.o: $(obj)/config.h
extra-y += xen_action_hooks_norevert.livepatch
xen_action_hooks_norevert-objs := xen_action_hooks_norevert.o xen_hello_world_func.o note.o xen_note.o
+$(obj)/xen_alternatives_fail.o: $(obj)/config.h
+
+extra-y += xen_alternatives_fail.livepatch
+xen_alternatives_fail-objs := xen_alternatives_fail.o note.o xen_note.o
+
EXPECT_BYTES_COUNT := 8
CODE_GET_EXPECT=$(shell $(OBJDUMP) -d --insn-width=1 $(1) | sed -n -e '/<'$(2)'>:$$/,/^$$/ p' | tail -n +2 | head -n $(EXPECT_BYTES_COUNT) | awk '{$$0=$$2; printf "%s", substr($$0,length-1)}' | sed 's/.\{2\}/0x&,/g' | sed 's/^/{/;s/,$$/}/g')
$(obj)/expect_config.h: $(objtree)/xen-syms
new file mode 100644
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2024 Cloud Software Group.
+ *
+ */
+
+#include "config.h"
+#include <xen/lib.h>
+#include <xen/livepatch_payload.h>
+
+#include <asm/alternative.h>
+#include <asm/cpuid.h>
+
+void test_alternatives(void)
+{
+ alternative("", "", NCAPINTS * 32);
+}
+
+/* Set a hook so the loading logic in Xen don't consider the payload empty. */
+LIVEPATCH_LOAD_HOOK(test_alternatives);
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
alternatives is used both at boot time, and when loading livepatch payloads. While for the former it makes sense to panic, it's not useful for the later, as for livepatches it's possible to fail to load the livepatch if alternatives cannot be resolved and continue operating normally. Relax the BUGs in _apply_alternatives() to instead return an error code if alternatives are applied after boot. Add a dummy livepatch test so the new logic can be assessed, with the change applied the loading now fails with: alt table ffff82d040602024 -> ffff82d040602032 livepatch: xen_alternatives_fail applying alternatives failed: -22 Signed-off-by: Roger Pau Monné <roge.rpau@citrix.com> --- xen/arch/x86/alternative.c | 29 ++++++++++++++++------ xen/arch/x86/include/asm/alternative.h | 3 ++- xen/common/livepatch.c | 10 +++++++- xen/test/livepatch/Makefile | 5 ++++ xen/test/livepatch/xen_alternatives_fail.c | 29 ++++++++++++++++++++++ 5 files changed, 66 insertions(+), 10 deletions(-) create mode 100644 xen/test/livepatch/xen_alternatives_fail.c