Re: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
On 17.03.22 11:00, Jiamei Xie wrote: -Original Message- From: Xen-devel On Behalf Of Jiamei Xie Sent: 2022年3月17日 17:17 To: Ross Lagerwall ; Bjoern Doebel ; xen-devel@lists.xenproject.org Cc: Michael Kurth ; Martin Pohlack ; Roger Pau Monne ; Andrew Cooper ; Konrad Rzeszutek Wilk Subject: RE: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET- enhanced functions Hi Bjoern, -Original Message- From: Xen-devel On Behalf Of Ross Lagerwall Sent: 2022年3月10日 1:12 To: Bjoern Doebel ; xen-devel@lists.xenproject.org Cc: Michael Kurth ; Martin Pohlack ; Roger Pau Monne ; Andrew Cooper ; Konrad Rzeszutek Wilk Subject: Re: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET- enhanced functions From: Bjoern Doebel Sent: Wednesday, March 9, 2022 2:53 PM To: xen-devel@lists.xenproject.org Cc: Michael Kurth ; Martin Pohlack ; Roger Pau Monne ; Andrew Cooper ; Bjoern Doebel ; Konrad Rzeszutek Wilk ; Ross Lagerwall Subject: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET- enhanced functions Xen enabled CET for supporting architectures. The control flow aspect of CET expects functions that can be called indirectly (i.e., via function pointers) to start with an ENDBR64 instruction. Otherwise a control flow exception is raised. This expectation breaks livepatching flows because we patch functions by overwriting their first 5 bytes with a JMP + , thus breaking the ENDBR64. We fix this by checking the start of a patched function for being ENDBR64. In the positive case we move the livepatch JMP to start behind the ENDBR64 instruction. To avoid having to guess the ENDBR64 offset again on patch reversal (which might race with other mechanisms adding/removing ENDBR dynamically), use the livepatch metadata to store the computed offset along with the saved bytes of the overwritten function. Signed-off-by: Bjoern Doebel Acked-by: Konrad Rzeszutek Wilk CC: Ross Lagerwall Reviewed-by: Ross Lagerwall Tested-by: Jiamei xie Cheers, Jiamei Sorry I forgot to add the scope I tested in last email. I tested it on armv8a. It worked fine and didn't break arm. Tested-by: Jiamei xie Thanks Jiamei! As Jan already pointed out there's a v6 patch out already. It is only cosmetically different from this one. Unless you insist, I'd not roll a v7 only to add this tag? Bjoern 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
Re: [PATCH 0/2] livepatch: fix handling of (some) relocations
On 17.03.22, 12:10, "Xen-devel on behalf of Roger Pau Monne" wrote: Hello, Relocations that reference symbols that belong to sections with a size of 0 are not properly resolved, as the address of those symbols won't be resolved in the first place. Fix this by not ignoring sections with a size of 0, while still properly handling the detection of whether a livepatch can be reapplied after being reverted (patch 1). Also detect whether any relocations reference unresolved symbols and error out in that case, as those relocations cannot be resolved (patch 2). I wonder whether it's possible to have unresolved symbols if we only ignore non SHF_ALLOC sections, so we could maybe error out earlier if we found a symbols that belongs to a non SHF_ALLOC section in livepatch_elf_resolve_symbols. The current approach is more conservative as we would only report an error if we have unresolved symbols that are referenced in relocations. Thanks, Roger. Roger Pau Monne (2): livepatch: do not ignore sections with 0 size livepatch: avoid relocations referencing ignored section symbols xen/arch/arm/arm32/livepatch.c | 7 +++ xen/arch/arm/arm64/livepatch.c | 7 +++ xen/arch/x86/livepatch.c| 7 +++ xen/common/livepatch.c | 16 +++- xen/common/livepatch_elf.c | 6 ++ xen/include/xen/livepatch_elf.h | 3 ++- 6 files changed, 40 insertions(+), 6 deletions(-) -- 2.34.1 I checked the x86 part and confirmed that my previously non-working livepatch loads now. Tested-by: Bjoern Doebel 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
Re: alternatives+livepatch testing
Hi,
Hello,
The recent hiccup with CET-IBT, and discovery that livepatch-build-tools
have been broken for several releases, demonstrates that we do not have
remotely adequate testing in place. We need to address, and ensure we
don't end up in the same position again.
Alternatives and Livepatching have a number of overlapping test
requirements, so how about the following plan:
1) Introduce a new $arch/scm-tests.c, with something akin to the
existing stub_selftest(). I'm tempted to move stub_selftest() out of
initcall and call it from init_done() (before we clobber .init.text)
because that gets shstk testing included.
Even without livepatching, we've got various requirements such as
endbr's only existing where expected, and getting clobbered when
suitably annotated, and altcalls turning into UD for a still-NULL pointer.
Items not yet upstream but on the radar include inlining of retpoline
thunks and SLS workarounds, which would also fit into this.
2) Provide (in xen.git) a patch to scm-tests.c which OSSTest/other can
use livepatch-build-tools on to generate a real livepatch, and a new
livepatching subop which can be invoked from xen-livepatch in userspace
that will run the same kind of consistency checks as 1) on the patched
content.
This lets us create specific constructs and confirm that they get
patched correctly, without having to specifically execute the result. I
(think) we can do everything needed without reference to the livepatch
metadata, which simplifies things.
Providing a patch isn't totally ideal from a "maintaining xen" point of
view, but I think we can have a build-time test which confirms the patch
is still good, and it is definitely the right primitive to use for the
end-to-end testing.
I can share a bit what we are doing with our internal livepatch testing:
* We have a simple test livepatch, similar to
xen/test/livepatch/xen_prepost_hooks.c. We build and load this livepatch
both in our nightly test setup as well as when testing production
builds. This test only validates livepatch functionality in the
hypervisor works and I think it would not have detected the build problem.
* To detect the build issue, we need an actual diff-based livepatch. We
are actually using a non-trivial patch here, which enables FEP in Xen at
load time, because we have tests that make use of FEP later on.
* If you are concerned about maintainability, I'd go for a very simple
diff to a location that won't change a lot. For my CET-IBT adaption, I
went for
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -405,6 +405,8 @@ static int cf_check vmx_domain_initialise(struct
domain *d)
.tail = vmx_do_resume,
};
+printk("vmx_domain_initialise called\n");
+
d->arch.ctxt_switch = &csw;
/*
==> This has the disadvantage of being arch-specific. There are likely
more generic locations in Xen that haven't been changed for years,
indicating likelihood of not requiring too many updates in the future.
* From a testing perspective it is a good idea to have a patch like the
one above with a visible side effect, because that allows you to later
also have testing that loads that actual patch and triggers the
functionality to observe the side effect.
So in summary, your plan sounds reasonable.
Bjoern
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
Re: [PATCH 3/3] livepatch: correctly handle altinstruction sections
On 10.03.22 16:08, Roger Pau Monne wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
The current handling of altinstructions sections by the livepatch
tools is incorrect, as on Xen those sections are part of .init and
thus discarded after load. Correctly handle them by just ignoring, as
it's done with other .init related sections.
While there also add .data.ro_after_init section as a read-only
section and introduce some syntactic sugar for comparing section
names.
Signed-off-by: Roger Pau Monné
---
I assume this handling of .altinstr* sections was inherited from Linux
where the sections are not discarded after load in order to apply
alternative patching to the loaded modules after boot.
---
common.c | 7 +--
create-diff-object.c | 26 --
2 files changed, 5 insertions(+), 28 deletions(-)
diff --git a/common.c b/common.c
index 68a71f7..a148d8a 100644
--- a/common.c
+++ b/common.c
@@ -249,19 +249,22 @@ int is_text_section(struct section *sec)
(sec->sh.sh_flags & SHF_EXECINSTR));
}
+#define SEC_MATCH(n) !strncmp(sec->name, n, strlen(n) - 1)
int is_rodata_section(struct section *sec)
{
return sec->sh.sh_type == SHT_PROGBITS &&
!(sec->sh.sh_flags & (SHF_EXECINSTR | SHF_WRITE)) &&
- !strncmp(sec->name, ".rodata", 7);
+ (SEC_MATCH(".rodata") || SEC_MATCH(".data.ro_after_init"));
}
int is_init_section(struct section *sec)
{
return sec->sh.sh_type == SHT_PROGBITS &&
(sec->sh.sh_flags & SHF_ALLOC) &&
- !strncmp(sec->name, ".init", 5);
+ (SEC_MATCH(".init") || SEC_MATCH(".text.startup") ||
+ SEC_MATCH(".altinstr") || SEC_MATCH(".ctors"));
}
+#undef SEC_MATCH
int is_debug_section(struct section *sec)
{
diff --git a/create-diff-object.c b/create-diff-object.c
index a516670..ec2afb4 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -995,19 +995,6 @@ static int ex_table_group_size(struct kpatch_elf *kelf,
int offset)
return size;
}
-static int altinstructions_group_size(struct kpatch_elf *kelf, int offset)
-{
- static int size = 0;
- char *str;
- if (!size) {
- str = getenv("ALT_STRUCT_SIZE");
- size = str ? atoi(str) : 12;
- }
-
- log_debug("altinstr_size=%d\n", size);
- return size;
-}
-
static int livepatch_hooks_group_size(struct kpatch_elf *kelf, int offset)
{
static int size = 0;
@@ -1021,11 +1008,6 @@ static int livepatch_hooks_group_size(struct kpatch_elf
*kelf, int offset)
return size;
}
-static int undefined_group_size(struct kpatch_elf *kelf, int offset)
-{
- return 0;
-}
-
/*
* The rela groups in the .fixup section vary in size. The beginning of each
* .fixup rela group is referenced by the .ex_table section. To find the size
@@ -1099,14 +1081,6 @@ static struct special_section special_sections[] = {
.name = ".ex_table",
.group_size = ex_table_group_size,
},
- {
- .name = ".altinstructions",
- .group_size = altinstructions_group_size,
- },
- {
- .name = ".altinstr_replacement",
- .group_size = undefined_group_size,
- },
{
.name = ".livepatch.hooks.load",
.group_size = livepatch_hooks_group_size,
--
2.34.1
Confirming, this solves the altsection issue I reported via
https://lore.kernel.org/xen-devel/b74a68b038c31df4bb94a5b5e87453f5a249cfe2.1646753657.git.doe...@amazon.de/
Reviewed-by: Bjoern Doebel
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
Re: [PATCH 2/3] livepatch: add extra efi/ objects to be ignored
On 10.03.22 16:08, Roger Pau Monne wrote: CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. The contents of this objects is init only, and cannot be patched. Signed-off-by: Roger Pau Monné --- livepatch-gcc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/livepatch-gcc b/livepatch-gcc index fe782e0..b0b9ce4 100755 --- a/livepatch-gcc +++ b/livepatch-gcc @@ -66,6 +66,8 @@ elif [[ "$TOOLCHAINCMD" =~ $OBJCOPY_RE ]] ; then version.o|\ debug.o|\ check.o|\ +boot.o|\ +*.init.o|\ .*.o) ;; *.o) -- 2.34.1 Reviewed-by: Bjoern Doebel 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
Re: [PATCH 1/3] livepatch: use basename to perform object file matching
On 10.03.22 16:08, Roger Pau Monne wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
The changes in the Xen build logic has resulted in the compiler and
objcopy being called from xen/ instead of relative to each object
directory. This requires using basename so that the directory is not
taken into account when checking against the list of files to be
explicitly ignored.
Also adjust the paths used to store the differing object files, as
with the current logic the resulting path will be wrong when using
newer Xen versions, changed_objs would end containing entries like:
xen/arch/x86/hvm/vmx/arch/x86/hvm/vmx/vmx.o
Signed-off-by: Roger Pau Monné
---
livepatch-gcc | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/livepatch-gcc b/livepatch-gcc
index 91333d5..fe782e0 100755
--- a/livepatch-gcc
+++ b/livepatch-gcc
@@ -32,10 +32,10 @@ if [[ "$TOOLCHAINCMD" =~ $GCC_RE ]] ; then
if [ "$1" = "-o" ]; then
obj=$2
[[ $2 = */.tmp_*.o ]] && obj=${2/.tmp_/}
-case "$obj" in
+case "$(basename $obj)" in
version.o|\
debug.o|\
-efi/check.o|\
+check.o|\
*.xen-syms.*.o|\
*.xen.efi.*.o|\
built_in.o|\
@@ -46,6 +46,7 @@ if [[ "$TOOLCHAINCMD" =~ $GCC_RE ]] ; then
*.o)
path="$(pwd)/$(dirname $obj)"
dir="${path#$LIVEPATCH_BUILD_DIR}"
+obj=$(basename $obj)
if [ -n "$LIVEPATCH_CAPTURE_DIR" -a -d
"$LIVEPATCH_CAPTURE_DIR" ]; then
echo "$dir/$obj" >>
"${LIVEPATCH_CAPTURE_DIR}/changed_objs"
keep=yes
@@ -61,15 +62,16 @@ if [[ "$TOOLCHAINCMD" =~ $GCC_RE ]] ; then
done
elif [[ "$TOOLCHAINCMD" =~ $OBJCOPY_RE ]] ; then
obj="${!#}"
-case "$obj" in
+case "$(basename $obj)" in
version.o|\
debug.o|\
-efi/check.o|\
+check.o|\
.*.o)
;;
*.o)
path="$(pwd)/$(dirname $obj)"
dir="${path#$LIVEPATCH_BUILD_DIR}"
+obj=$(basename $obj)
if [ -n "$LIVEPATCH_CAPTURE_DIR" -a -d "$LIVEPATCH_CAPTURE_DIR"
]; then
echo "$dir/$obj" >> "${LIVEPATCH_CAPTURE_DIR}/changed_objs"
keep=yes
@@ -85,7 +87,7 @@ ret="$?"
if [[ "$keep" = "yes" ]] ; then
mkdir -p "$(dirname $LIVEPATCH_CAPTURE_DIR/$dir/$obj)"
-cp "$obj" "$LIVEPATCH_CAPTURE_DIR/$dir/$obj"
+cp "$path/$obj" "$LIVEPATCH_CAPTURE_DIR/$dir/$obj"
fi
exit "$ret"
--
2.34.1
Reviewed-by: Bjoern Doebel
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
Re: [PATCH] x86/cet: Use dedicated NOP4 for cf_clobber
On 08.03.22 15:01, Andrew Cooper wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
For livepatching, we need to look at a potentially clobbered function and
determine whether it used to have an ENDBR64 instruction.
Use a non-default 4-byte P6 long nop, not emitted by toolchains, and introduce
the was_endbr64() predicate.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Wei Liu
CC: Bjoern Doebel
CC: Michael Kurth
CC: Martin Pohlack
Bjoern: For the livepatching code, I think you want:
if ( is_endbr64(...) || was_endbr64(...) )
needed += ENDBR64_LEN;
---
xen/arch/x86/alternative.c | 10 +-
xen/arch/x86/include/asm/endbr.h | 12
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index d41eeef1bcaf..ffb1b1d960c8 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -362,7 +362,15 @@ static void init_or_livepatch _apply_alternatives(struct
alt_instr *start,
if ( !is_kernel_text(ptr) || !is_endbr64(ptr) )
continue;
-add_nops(ptr, ENDBR64_LEN);
+/*
+ * Can't use add_nops() here. ENDBR64_POISON is specifically
+ * different to NOP4 so it can be spotted after the fact.
+ *
+ * All CET-capable hardware uses P6 NOPS (no need to plumb through
+ * ideal_nops), and doesn't require a branch to synchronise the
+ * instruction stream.
+ */
+memcpy(ptr, ENDBR64_POISON, ENDBR64_LEN);
clobbered++;
}
diff --git a/xen/arch/x86/include/asm/endbr.h b/xen/arch/x86/include/asm/endbr.h
index 6090afeb0bd8..5e1e55cb467d 100644
--- a/xen/arch/x86/include/asm/endbr.h
+++ b/xen/arch/x86/include/asm/endbr.h
@@ -52,4 +52,16 @@ static inline void place_endbr64(void *ptr)
*(uint32_t *)ptr = gen_endbr64();
}
+/*
+ * After clobbering ENDBR64, we may need to confirm that the site used to
+ * contain an ENDBR64 instruction. Use an encoding which isn't the default
+ * P6_NOP4.
+ */
+#define ENDBR64_POISON "\x66\x0f\x1f\x00" /* osp nopl (%rax) */
+
+static inline bool was_endbr64(const void *ptr)
+{
+return *(const uint32_t *)ptr == 0x001f0f66;
+}
+
#endif /* XEN_ASM_ENDBR_H */
--
2.11.0
Reviewed-by: Bjoern Doebel
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
Re: [PATCH v3 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
On 08.03.22 17:01, Ross Lagerwall wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
From: Doebel, Bjoern
Sent: Tuesday, March 8, 2022 3:41 PM
To: Ross Lagerwall ; xen-devel@lists.xenproject.org
Cc: Michael Kurth ; Martin Pohlack ; Roger Pau Monne
; Andrew Cooper ; Konrad Rzeszutek Wilk
Subject: Re: [PATCH v3 2/2] xen/x86: Livepatch: support patching CET-enhanced
functions
On 08.03.22 16:25, Ross Lagerwall wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
From: Bjoern Doebel
Sent: Tuesday, March 8, 2022 10:29 AM
To: xen-devel@lists.xenproject.org
Cc: Michael Kurth ; Martin Pohlack ; Roger Pau Monne
; Andrew Cooper ; Bjoern Doebel ;
Konrad Rzeszutek Wilk ; Ross Lagerwall
Subject: [PATCH v3 2/2] xen/x86: Livepatch: support patching CET-enhanced
functions
Xen enabled CET for supporting architectures. The control flow aspect of
CET expects functions that can be called indirectly (i.e., via function
pointers) to start with an ENDBR64 instruction. Otherwise a control flow
exception is raised.
This expectation breaks livepatching flows because we patch functions by
overwriting their first 5 bytes with a JMP + , thus breaking the
ENDBR64. We fix this by checking the start of a patched function for
being ENDBR64. In the positive case we move the livepatch JMP to start
behind the ENDBR64 instruction.
To avoid having to guess the ENDBR64 offset again on patch reversal
(which might race with other mechanisms adding/removing ENDBR
dynamically), use the livepatch metadata to store the computed offset
along with the saved bytes of the overwritten function.
Signed-off-by: Bjoern Doebel
CC: Konrad Rzeszutek Wilk
CC: Ross Lagerwall
Note that on top of livepatching functions, Xen supports an additional
mode where we can "remove" a function by overwriting it with NOPs. This
is only supported for functions up to 31 bytes in size and this patch
reduces this limit to 30 bytes.
Changes since r1:
* use sizeof_field() to avoid unused variable warning
* make metadata variable const in arch_livepatch_revert
---
xen/arch/x86/livepatch.c | 61 ++--
1 file changed, 53 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 65530c1e57..0fd97f2a00 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -14,11 +14,29 @@
#include
#include
+#include
#include
#include
#include
#include
+/*
+ * CET hotpatching support: We may have functions starting with an ENDBR64
+ * instruction that MUST remain the first instruction of the function, hence
+ * we need to move any hotpatch trampoline further into the function. For that
+ * we need to keep track of the patching offset used for any loaded hotpatch
+ * (to avoid racing against other fixups adding/removing ENDBR64 or similar
+ * instructions).
+ *
+ * We do so by making use of the existing opaque metadata area. We use its
+ * first 4 bytes to track the offset into the function used for patching and
+ * the remainder of the data to store overwritten code bytes.
+ */
+struct x86_livepatch_meta {
+uint8_t patch_offset;
+uint8_t instruction[LIVEPATCH_OPAQUE_SIZE - sizeof(uint8_t)];
+};
+
I think it would make things a bit simpler to use one of the spare _pad bits
from struct livepatch_func ather than hacking it into the opaque area. Is
there a reason you chose this approach?
No specific reason. Are you suggesting updating the public livepatch
interface to add a new member and reduce the padding size by 1 byte? I
guess that will also require a patch to livepatch-build-tools?
Bjoern
struct livepatch_func contains info that the build tool needs to
communicate to Xen as well as space for Xen's internal book keeping
while the live patch is applied. This includes the array for storing
instructions, the applied flag, and now additionally the patch offset.
(It's a somewhat odd arrangement but it's what we've got...)
The build tool does not need to know the details about any of Xen's internal
book keeping. So my preference would be to have patch_offset as an additional
member next to applied (reducing padding by 1) and then in livepatch-build-tools
replace:
unsigned char pad[MAX_REPLACEMENT_SIZE];
uint8_t applied;
uint8_t _pad[7];
with simply:
uint8_t opaque[39];
What do you think?
That will simplify this patch - I like it. Will send update +
livepatch-build patch.
Bjoern
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
Re: [PATCH v3 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
On 08.03.22 16:25, Ross Lagerwall wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
From: Bjoern Doebel
Sent: Tuesday, March 8, 2022 10:29 AM
To: xen-devel@lists.xenproject.org
Cc: Michael Kurth ; Martin Pohlack ; Roger Pau Monne
; Andrew Cooper ; Bjoern Doebel ;
Konrad Rzeszutek Wilk ; Ross Lagerwall
Subject: [PATCH v3 2/2] xen/x86: Livepatch: support patching CET-enhanced
functions
Xen enabled CET for supporting architectures. The control flow aspect of
CET expects functions that can be called indirectly (i.e., via function
pointers) to start with an ENDBR64 instruction. Otherwise a control flow
exception is raised.
This expectation breaks livepatching flows because we patch functions by
overwriting their first 5 bytes with a JMP + , thus breaking the
ENDBR64. We fix this by checking the start of a patched function for
being ENDBR64. In the positive case we move the livepatch JMP to start
behind the ENDBR64 instruction.
To avoid having to guess the ENDBR64 offset again on patch reversal
(which might race with other mechanisms adding/removing ENDBR
dynamically), use the livepatch metadata to store the computed offset
along with the saved bytes of the overwritten function.
Signed-off-by: Bjoern Doebel
CC: Konrad Rzeszutek Wilk
CC: Ross Lagerwall
Note that on top of livepatching functions, Xen supports an additional
mode where we can "remove" a function by overwriting it with NOPs. This
is only supported for functions up to 31 bytes in size and this patch
reduces this limit to 30 bytes.
Changes since r1:
* use sizeof_field() to avoid unused variable warning
* make metadata variable const in arch_livepatch_revert
---
xen/arch/x86/livepatch.c | 61 ++--
1 file changed, 53 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 65530c1e57..0fd97f2a00 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -14,11 +14,29 @@
#include
#include
+#include
#include
#include
#include
#include
+/*
+ * CET hotpatching support: We may have functions starting with an ENDBR64
+ * instruction that MUST remain the first instruction of the function, hence
+ * we need to move any hotpatch trampoline further into the function. For that
+ * we need to keep track of the patching offset used for any loaded hotpatch
+ * (to avoid racing against other fixups adding/removing ENDBR64 or similar
+ * instructions).
+ *
+ * We do so by making use of the existing opaque metadata area. We use its
+ * first 4 bytes to track the offset into the function used for patching and
+ * the remainder of the data to store overwritten code bytes.
+ */
+struct x86_livepatch_meta {
+uint8_t patch_offset;
+uint8_t instruction[LIVEPATCH_OPAQUE_SIZE - sizeof(uint8_t)];
+};
+
I think it would make things a bit simpler to use one of the spare _pad bits
from struct livepatch_func ather than hacking it into the opaque area. Is
there a reason you chose this approach?
No specific reason. Are you suggesting updating the public livepatch
interface to add a new member and reduce the padding size by 1 byte? I
guess that will also require a patch to livepatch-build-tools?
Bjoern
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
Re: [PATCH v3 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
On 08.03.22 14:06, Konrad Rzeszutek Wilk wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
On Tue, Mar 08, 2022 at 12:44:54PM +, Andrew Cooper wrote:
On 08/03/2022 10:29, Bjoern Doebel wrote:
@@ -104,18 +122,34 @@ void noinline arch_livepatch_revive(void)
int arch_livepatch_verify_func(const struct livepatch_func *func)
{
+BUILD_BUG_ON(sizeof(struct x86_livepatch_meta) != LIVEPATCH_OPAQUE_SIZE);
+
/* If NOPing.. */
if ( !func->new_addr )
{
/* Only do up to maximum amount we can put in the ->opaque. */
-if ( func->new_size > sizeof(func->opaque) )
+if ( func->new_size > sizeof_field(struct x86_livepatch_meta,
+ instruction) )
return -EOPNOTSUPP;
if ( func->old_size < func->new_size )
return -EINVAL;
}
-else if ( func->old_size < ARCH_PATCH_INSN_SIZE )
-return -EINVAL;
+else
+{
+/*
+ * Space needed now depends on whether the target function
+ * starts with an ENDBR64 instruction.
+ */
+uint8_t needed;
+
+needed = ARCH_PATCH_INSN_SIZE;
+if ( is_endbr64(func->old_addr) )
+needed += ENDBR64_LEN;
This won't work for cf_clobber targets, I don't think. The ENDBR gets
converted to NOP4 and fails this check, but the altcalls calling
old_func had their displacements adjusted by +4.
The is_endbr64() check will fail, and the 5-byte jmp will be written at
the start of the function, and corrupt the instruction stream for the
altcall()'d callers.
Let me write an incremental patch to help.
Please add Acked-by: Konrad Rzeszutek Wilk
on the patches.
Thanks, will do!
Bjoern
Thank you
~Andrew
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
Re: [PATCH v3 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
On 08.03.22 13:44, Andrew Cooper wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
On 08/03/2022 10:29, Bjoern Doebel wrote:
@@ -104,18 +122,34 @@ void noinline arch_livepatch_revive(void)
int arch_livepatch_verify_func(const struct livepatch_func *func)
{
+BUILD_BUG_ON(sizeof(struct x86_livepatch_meta) != LIVEPATCH_OPAQUE_SIZE);
+
/* If NOPing.. */
if ( !func->new_addr )
{
/* Only do up to maximum amount we can put in the ->opaque. */
-if ( func->new_size > sizeof(func->opaque) )
+if ( func->new_size > sizeof_field(struct x86_livepatch_meta,
+ instruction) )
return -EOPNOTSUPP;
if ( func->old_size < func->new_size )
return -EINVAL;
}
-else if ( func->old_size < ARCH_PATCH_INSN_SIZE )
-return -EINVAL;
+else
+{
+/*
+ * Space needed now depends on whether the target function
+ * starts with an ENDBR64 instruction.
+ */
+uint8_t needed;
+
+needed = ARCH_PATCH_INSN_SIZE;
+if ( is_endbr64(func->old_addr) )
+needed += ENDBR64_LEN;
This won't work for cf_clobber targets, I don't think. The ENDBR gets
converted to NOP4 and fails this check, but the altcalls calling
old_func had their displacements adjusted by +4.
The is_endbr64() check will fail, and the 5-byte jmp will be written at
the start of the function, and corrupt the instruction stream for the
altcall()'d callers.
Let me write an incremental patch to help.
Thanks. Will you be adding a
memcmp(func->old_addr, ideal_nops[4], 4)
or is that once more too naive?
Bjoern
~Andrew
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
Re: [PATCH v2 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
On 08.03.22 09:07, Jan Beulich wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
On 07.03.2022 22:13, Bjoern Doebel wrote:
@@ -159,7 +200,11 @@ void noinline arch_livepatch_apply(struct livepatch_func
*func)
*/
void noinline arch_livepatch_revert(const struct livepatch_func *func)
{
-memcpy(func->old_addr, func->opaque, livepatch_insn_len(func));
+struct x86_livepatch_meta *lp;
Repeating my comment here a 3rd time (sorry): const please (and
generally wherever possible).
That's embarrassing... ;) Sorry. Will fix.
Bjoern
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
Re: [PATCH 1/1] xen/x86: Livepatch: support patching CET-enhanced functions
Please discard this accidental copy of patch 2/2.
On 07.03.22 12:53, Bjoern Doebel wrote:
Xen enabled CET for supporting architectures. The control flow aspect of
CET expects functions that can be called indirectly (i.e., via function
pointers) to start with an ENDBR64 instruction. Otherwise a control flow
exception is raised.
This expectation breaks livepatching flows because we patch functions by
overwriting their first 5 bytes with a JMP + , thus breaking the
ENDBR64. We fix this by checking the start of a patched function for
being ENDBR64. In the positive case we move the livepatch JMP to start
behind the ENDBR64 instruction.
To avoid having to guess the ENDBR64 offset again on patch reversal
(which might race with other mechanisms adding/removing ENDBR
dynamically), use the livepatch metadata to store the computed offset
along with the saved bytes of the overwritten function.
Signed-off-by: Bjoern Doebel
CC: Konrad Rzeszutek Wilk
CC: Ross Lagerwall
Note that on top of livepatching functions, Xen supports an additional
mode where we can "remove" a function by overwriting it with NOPs. This
is only supported for functions up to 31 bytes in size and this patch
reduces this limit to 30 bytes.
---
xen/arch/x86/livepatch.c | 63 +++-
1 file changed, 55 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 65530c1e57..da7611c01d 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -14,11 +14,29 @@
#include
#include
+#include
#include
#include
#include
#include
+/*
+ * CET hotpatching support: We may have functions starting with an ENDBR64
+ * instruction that MUST remain the first instruction of the function, hence
+ * we need to move any hotpatch trampoline further into the function. For that
+ * we need to keep track of the patching offset used for any loaded hotpatch
+ * (to avoid racing against other fixups adding/removing ENDBR64 or similar
+ * instructions).
+ *
+ * We do so by making use of the existing opaque metadata area. We use its
+ * first 4 bytes to track the offset into the function used for patching and
+ * the remainder of the data to store overwritten code bytes.
+ */
+struct x86_livepatch_meta {
+uint8_t patch_offset;
+uint8_t instruction[LIVEPATCH_OPAQUE_SIZE - sizeof(uint8_t)];
+};
+
static bool has_active_waitqueue(const struct vm_event_domain *ved)
{
/* ved may be xzalloc()'d without INIT_LIST_HEAD() yet. */
@@ -104,18 +122,36 @@ void noinline arch_livepatch_revive(void)
int arch_livepatch_verify_func(const struct livepatch_func *func)
{
+BUILD_BUG_ON(sizeof(struct x86_livepatch_meta) != LIVEPATCH_OPAQUE_SIZE);
+
/* If NOPing.. */
if ( !func->new_addr )
{
+struct x86_livepatch_meta *lp;
+
+lp = (struct x86_livepatch_meta *)func->opaque;
/* Only do up to maximum amount we can put in the ->opaque. */
-if ( func->new_size > sizeof(func->opaque) )
+if ( func->new_size > sizeof(lp->instruction) )
return -EOPNOTSUPP;
if ( func->old_size < func->new_size )
return -EINVAL;
}
-else if ( func->old_size < ARCH_PATCH_INSN_SIZE )
-return -EINVAL;
+else
+{
+/*
+ * Space needed now depends on whether the target function
+ * starts with an ENDBR64 instruction.
+ */
+uint8_t needed;
+
+needed = ARCH_PATCH_INSN_SIZE;
+if ( is_endbr64(func->old_addr) )
+needed += ENDBR64_LEN;
+
+if ( func->old_size < needed )
+return -EINVAL;
+}
return 0;
}
@@ -127,15 +163,21 @@ int arch_livepatch_verify_func(const struct
livepatch_func *func)
void noinline arch_livepatch_apply(struct livepatch_func *func)
{
uint8_t *old_ptr;
-uint8_t insn[sizeof(func->opaque)];
+struct x86_livepatch_meta *lp;
+uint8_t insn[sizeof(lp->instruction)];
unsigned int len;
+lp = (struct x86_livepatch_meta *)func->opaque;
+lp->patch_offset = 0;
old_ptr = func->old_addr;
len = livepatch_insn_len(func);
if ( !len )
return;
-memcpy(func->opaque, old_ptr, len);
+if ( is_endbr64(old_ptr) )
+lp->patch_offset += ENDBR64_LEN;
+
+memcpy(lp->instruction, old_ptr + lp->patch_offset, len);
if ( func->new_addr )
{
int32_t val;
@@ -143,14 +185,15 @@ void noinline arch_livepatch_apply(struct livepatch_func
*func)
BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE != (1 + sizeof(val)));
insn[0] = 0xe9; /* Relative jump. */
-val = func->new_addr - func->old_addr - ARCH_PATCH_INSN_SIZE;
+val = func->new_addr - (func->old_addr + lp->patch_offset
++ ARCH_PATCH_INSN_SIZE);
memcpy(&insn[1], &val, sizeof(val));
}
else
add_nops(insn, len);
-memcpy(old_ptr,
Re: [PATCH] xen/x86: Livepatch: support patching CET-enhanced functions
On 07.03.22 10:37, Jan Beulich wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
On 07.03.2022 10:17, Bjoern Doebel wrote:
Xen enabled CET for supporting architectures. The control flow aspect of
CET expects functions that can be called indirectly (i.e., via function
pointers) to start with an ENDBR64 instruction. Otherwise a control flow
exception is raised.
This expectation breaks livepatching flows because we patch functions by
overwriting their first 5 bytes with a JMP + , thus breaking the
ENDBR64. We fix this by checking the start of a patched function for
being ENDBR64. In the positive case we move the livepatch JMP to start
behind the ENDBR64 instruction.
To avoid having to guess the ENDBR64 offset again on patch reversal
(which might race with other mechanisms adding/removing ENDBR
dynamically), use the livepatch metadata to store the computed offset
along with the saved bytes of the overwritten function.
Signed-off-by: Bjoern Doebel
CC: Konrad Rzeszutek Wilk
CC: Ross Lagerwall
Note that on top of livepatching functions, Xen supports an additional
mode where we can "remove" a function by overwriting it with NOPs. This
is only supported for functions up to 31 bytes in size and this patch
reduces this limit to 27 bytes.
Is this intended to be part of the description (then it should move up)
or a post-commit-message remark (then there should be a --- separator
ahead of it)?
Missing separator. Will fix as well as your style comments below.
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -14,11 +14,28 @@
#include
#include
+#include
#include
#include
#include
#include
+/*
+ * CET hotpatching support: We may have functions starting with an ENDBR64
instruction
+ * that MUST remain the first instruction of the function, hence we need to
move any
+ * hotpatch trampoline further into the function. For that we need to keep
track of the
+ * patching offset used for any loaded hotpatch (to avoid racing against other
fixups
+ * adding/removing ENDBR64 or similar instructions).
+ *
+ * We do so by making use of the existing opaque metadata area. We use its
first 4 bytes
+ * to track the offset into the function used for patching and the remainder
of the data
+ * to store overwritten code bytes.
+ */
Style: Line length (also elsewhere).
+struct __packed x86_livepatch_meta {
+int32_t patch_offset;
I was first going to suggest to use plain int here to comply with
./CODING_STYLE, but it looks like int8_t or uint8_t will do, leaving
more space for the insn. I'm also not convinced you really need the
__packed annotation. Furthermore, to avoid the need for casts,
simply using the ->opaque[] directly would look to be an option then
(with suitable #define-s to identify the two parts).
* Agree on the uint8_t.
* The __packed was mostly to be really sure there's no added padding.
Likely will be needed once I change the type for the offset.
* I think using an explicit struct is more readable+comprehensible than
magically defining macros to reinterpret the opaque[] array, so I'd
prefer that. No hard feelings, though.
@@ -104,11 +121,14 @@ void noinline arch_livepatch_revive(void)
int arch_livepatch_verify_func(const struct livepatch_func *func)
{
+BUILD_BUG_ON(sizeof(struct x86_livepatch_meta) != LIVEPATCH_OPAQUE_SIZE);
+
/* If NOPing.. */
if ( !func->new_addr )
{
+struct x86_livepatch_meta *lp = (struct
x86_livepatch_meta*)func->opaque;
_If_ there is to remain a struct, please insert the missing blank before
the star (applicable elsewhere as well), and please separate declaration(s)
from statement(s) by a blank line.
-if ( func->new_size > sizeof(func->opaque) )
+if ( func->new_size > sizeof(lp->instruction) )
This being the only use of the new variable, I expect compilers to warn
about the variable being actually unused.
@@ -127,15 +147,20 @@ int arch_livepatch_verify_func(const struct
livepatch_func *func)
void noinline arch_livepatch_apply(struct livepatch_func *func)
{
uint8_t *old_ptr;
-uint8_t insn[sizeof(func->opaque)];
+struct x86_livepatch_meta *lp = (struct x86_livepatch_meta*)func->opaque;
+uint8_t insn[sizeof(lp->instruction)];
unsigned int len;
+lp->patch_offset = 0;
old_ptr = func->old_addr;
len = livepatch_insn_len(func);
if ( !len )
return;
-memcpy(func->opaque, old_ptr, len);
+if ( is_endbr64( old_ptr ) )
Style: No blanks inside the inner parentheses, please.
@@ -159,7 +184,9 @@ void noinline arch_livepatch_apply(struct livepatch_func
*func)
*/
void noinline arch_livepatch_revert(const struct livepatch_func *func)
{
-memcpy(func->old_addr, func->opaque, livepatch_insn_len(func));
+struct x86_livepatch_meta *lp = (struct x86_livepatch_meta*)func->opaq
Re: [PATCH 4/4] livepatch: differentiate between old and new build systems
On 02.03.22 15:27, Roger Pau Monne wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
Do not attempt to modify the build system if CFLAGS are not set in
Rules.mk, and instead rely on CONFIG_LIVEPATCH already setting
-f{function,data}-sections.
Signed-off-by: Roger Pau Monné
---
This depends on getting the patch to add -f{function,data}-sections
when using CONFIG_LIVEPATCH accepted.
---
livepatch-build | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/livepatch-build b/livepatch-build
index 38a92be..656cdac 100755
--- a/livepatch-build
+++ b/livepatch-build
@@ -98,14 +98,20 @@ function build_special()
# Build with special GCC flags
cd "${SRCDIR}/xen" || die
-sed -i 's/CFLAGS += -nostdinc/CFLAGS += -nostdinc -ffunction-sections
-fdata-sections/' Rules.mk
-cp -p arch/x86/Makefile arch/x86/Makefile.bak
-sed -i 's/--section-alignment=0x20/--section-alignment=0x1000/'
arch/x86/Makefile
-# Restore timestamps to prevent spurious rebuilding
-touch --reference=arch/x86/Makefile.bak arch/x86/Makefile
-make "-j$CPUS" $XEN_DEBUG &> "${OUTPUT}/build_${name}_compile.log" || die
-sed -i 's/CFLAGS += -nostdinc -ffunction-sections -fdata-sections/CFLAGS
+= -nostdinc/' Rules.mk
-mv -f arch/x86/Makefile.bak arch/x86/Makefile
+if grep -q 'nostdinc' Rules.mk; then
+ # Support for old build system, attempt to set
-f{function,data}-sections and rebuild
+sed -i 's/CFLAGS += -nostdinc/CFLAGS += -nostdinc -ffunction-sections
-fdata-sections/' Rules.mk
+cp -p arch/x86/Makefile arch/x86/Makefile.bak
+sed -i 's/--section-alignment=0x20/--section-alignment=0x1000/'
arch/x86/Makefile
+# Restore timestamps to prevent spurious rebuilding
+touch --reference=arch/x86/Makefile.bak arch/x86/Makefile
+make "-j$CPUS" $XEN_DEBUG &> "${OUTPUT}/build_${name}_compile.log" ||
die
+sed -i 's/CFLAGS += -nostdinc -ffunction-sections
-fdata-sections/CFLAGS += -nostdinc/' Rules.mk
+mv -f arch/x86/Makefile.bak arch/x86/Makefile
+else
+# -f{function,data}-sections set by CONFIG_LIVEPATCH
+make "-j$CPUS" $XEN_DEBUG &> "${OUTPUT}/build_${name}_compile.log" ||
die
+fi
unset LIVEPATCH_BUILD_DIR
unset LIVEPATCH_CAPTURE_DIR
Reviewed-by: Bjoern Doebel
Confirming that I can build a livepatch against upstream master with
your series and the other CONFIG_LIVEPATCH patch.
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
Re: [PATCH 3/4] livepatch: do the initial build using CROSS_COMPILE
On 02.03.22 15:27, Roger Pau Monne wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
Setting it afterwards for further builds will cause the build logic to
detect a change and thus force a rebuild of all sources.
Signed-off-by: Roger Pau Monné
---
livepatch-build | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/livepatch-build b/livepatch-build
index e1715ea..38a92be 100755
--- a/livepatch-build
+++ b/livepatch-build
@@ -92,7 +92,6 @@ function build_special()
cd "${SRCDIR}" || die
# Capture .o files from the patched build
-export CROSS_COMPILE="${TOOLSDIR}/livepatch-gcc "
export LIVEPATCH_BUILD_DIR="$(pwd)/"
export LIVEPATCH_CAPTURE_DIR="$OUTPUT/${name}"
mkdir -p "$LIVEPATCH_CAPTURE_DIR"
@@ -408,6 +407,8 @@ if [ "${SKIP}" != "build" ]; then
XEN_DEBUG="debug=$XEN_DEBUG"
fi
+export CROSS_COMPILE="${TOOLSDIR}/livepatch-gcc "
+
echo "Perform full initial build with ${CPUS} CPU(s)..."
build_full
Reviewed-by: Bjoern Doebel
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
Re: [PATCH 4/4] livepatch: differentiate between old and new build systems
On 02.03.22 15:27, Roger Pau Monne wrote:
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
Do not attempt to modify the build system if CFLAGS are not set in
Rules.mk, and instead rely on CONFIG_LIVEPATCH already setting
-f{function,data}-sections.
Signed-off-by: Roger Pau Monné
---
This depends on getting the patch to add -f{function,data}-sections
when using CONFIG_LIVEPATCH accepted.
---
livepatch-build | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/livepatch-build b/livepatch-build
index 38a92be..656cdac 100755
--- a/livepatch-build
+++ b/livepatch-build
@@ -98,14 +98,20 @@ function build_special()
# Build with special GCC flags
cd "${SRCDIR}/xen" || die
-sed -i 's/CFLAGS += -nostdinc/CFLAGS += -nostdinc -ffunction-sections
-fdata-sections/' Rules.mk
-cp -p arch/x86/Makefile arch/x86/Makefile.bak
-sed -i 's/--section-alignment=0x20/--section-alignment=0x1000/'
arch/x86/Makefile
-# Restore timestamps to prevent spurious rebuilding
-touch --reference=arch/x86/Makefile.bak arch/x86/Makefile
-make "-j$CPUS" $XEN_DEBUG &> "${OUTPUT}/build_${name}_compile.log" || die
-sed -i 's/CFLAGS += -nostdinc -ffunction-sections -fdata-sections/CFLAGS
+= -nostdinc/' Rules.mk
-mv -f arch/x86/Makefile.bak arch/x86/Makefile
+if grep -q 'nostdinc' Rules.mk; then
This means we are still breaking livepatch builds for all Xen versions
between your recent patch and the original patch that moved CFLAGS to
xen/Makefile (commit 2740d96efdd3009f8adb40aacdbcf05cfe8d1bbb, Fri Apr
24 14:57:10 2020 +0200).
Is this acceptable? (I mean, no one seems to have noticed...)
+ # Support for old build system, attempt to set
-f{function,data}-sections and rebuild
+sed -i 's/CFLAGS += -nostdinc/CFLAGS += -nostdinc -ffunction-sections
-fdata-sections/' Rules.mk
+cp -p arch/x86/Makefile arch/x86/Makefile.bak
+sed -i 's/--section-alignment=0x20/--section-alignment=0x1000/'
arch/x86/Makefile
+# Restore timestamps to prevent spurious rebuilding
+touch --reference=arch/x86/Makefile.bak arch/x86/Makefile
+make "-j$CPUS" $XEN_DEBUG &> "${OUTPUT}/build_${name}_compile.log" ||
die
+sed -i 's/CFLAGS += -nostdinc -ffunction-sections
-fdata-sections/CFLAGS += -nostdinc/' Rules.mk
+mv -f arch/x86/Makefile.bak arch/x86/Makefile
+else
+# -f{function,data}-sections set by CONFIG_LIVEPATCH
+make "-j$CPUS" $XEN_DEBUG &> "${OUTPUT}/build_${name}_compile.log" ||
die
+fi
unset LIVEPATCH_BUILD_DIR
unset LIVEPATCH_CAPTURE_DIR
Bjoern
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
Re: [PATCH 2/4] livepatch: improve rune for fetching of Build ID
The current one is broken with my version of readelf and returns
'NT_GNU_BUILD_ID'.
Signed-off-by: Roger Pau Monné
---
README.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/README.md b/README.md
index b48a3df..948a7de 100644
--- a/README.md
+++ b/README.md
@@ -16,7 +16,7 @@ $ cp -r ~/src/xen ~/src/xenbuild
$ cd ~/src/xen/xen
$ make nconfig # Make sure to set CONFIG_LIVEPATCH=y
$ make
-$ BUILDID=$(readelf -Wn xen-syms | awk '/Build ID:/ {print $3}')
+$ BUILDID=$(readelf -Wn xen-syms | sed -n -e 's/^.*Build ID: //p')
```
Next, build a live patch, using a patch and the source, build ID, and
Reviewed-by: Bjoern Doebel
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
Re: [PATCH 1/4] livepatch: update readme to mention --xen-depends
Fixes: b19df7b2c05e ('livepatch-build: Embed hypervisor build id into every
hotpatch')
Signed-off-by: Roger Pau Monné
---
README.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/README.md b/README.md
index 653c624..b48a3df 100644
--- a/README.md
+++ b/README.md
@@ -24,7 +24,7 @@ Next, build a live patch, using a patch and the source,
build ID, and
```
$ cd ~/src/livepatch-build
$ ./livepatch-build -s ~/src/xenbuild -p ~/src/xsa.patch -o out \
--c ~/src/xen/xen/.config --depends $BUILDID
+-c ~/src/xen/xen/.config --depends $BUILDID --xen-depends $BUILDID
Building LivePatch patch: xsa
Reviewed-by: Bjoern Doebel
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
