|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v2 3/5] xen/livepatch/ARM32: Don't load and crash on livepatches loaded with wrong alignment.
This issue was observed on ARM32 with a cross compiler for the
livepatches. Mainly the livepatches .data section size was not
padded to the section alignment:
ARM32 native:
Contents of section .rodata:
0000 68695f66 756e6300 63686563 6b5f666e hi_func.check_fn
0010 63000000 78656e5f 65787472 615f7665 c...xen_extra_ve
0020 7273696f 6e000000 rsion...
ARM32 cross compiler:
Contents of section .rodata:
0000 68695f66 756e6300 63686563 6b5f666e hi_func.check_fn
0010 63000000 78656e5f 65787472 615f7665 c...xen_extra_ve
0020 7273696f 6e00 rsion.
And when we loaded it the next section would be put right behind it:
native:
(XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .text at 00a02000
(XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .rodata at 00a04024
(XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .altinstructions at
00a0404c
cross compiler:
(XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .text at 00a02000
(XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .rodata at 00a04024
(XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .altinstructions at
00a0404a
(See 4a vs 4c)
native readelf:
[ 4] .rodata PROGBITS 00000000 000164 000028 00 A 0 0 4
[ 5] .altinstructions PROGBITS 00000000 00018c 00000c 00 A 0 0 1
cross compiler readelf --sections:
[ 4] .rodata PROGBITS 00000000 000164 000026 00 A 0 0 4
[ 5] .altinstructions PROGBITS 00000000 00018a 00000c 00 A 0 0 1
And as can be seen the .altinstructions have alignment of 1 which from
'man elf' is: "Values of zero and one mean no alignment is required."
which means we can ignore it.
See patch titled "alternative/x86/arm32: Align altinstructions
(and altinstr_replacement) sections." which fixes the .altinstructions to
have the proper alignment.
Ignoring this will result in a crash as when we started processing
".rel.altinstructions" for ".altinstructions" with a cross-compiled payload
we would end up poking in an section that was not aligned properly in memory
and crash.
This allows us on ARM32 to error out with:
livepatch: xen_hello_world: dest=00a0404a (.altinstructions) is not aligned
properly!
Furthermore we also observe that the alignment is not correct
for other sections (when cross compiling) as such we add the check
in various other places which allows us to get.
livepatch: xen_bye_world: .livepatch.depends alignment is wrong!
instead of a crash.
(See patch "xen/livepatch/x86/arm32: Force livepatch_depends to be uint32_t
aligned"
which fixes the .livepatch.depends alignment)
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
v1: Initial patch
v2: Redo the commit to include the commits which fix the alignment issues.
Also mention the need in the docs
---
docs/misc/livepatch.markdown | 6 +++++-
xen/arch/arm/arm32/livepatch.c | 15 +++++++++++++++
xen/arch/arm/arm64/livepatch.c | 6 ++++++
xen/arch/x86/livepatch.c | 6 ++++++
xen/common/livepatch.c | 14 +++++++++++++-
xen/include/xen/livepatch.h | 1 +
6 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown
index 54a6b850cb..c2c4acd3d3 100644
--- a/docs/misc/livepatch.markdown
+++ b/docs/misc/livepatch.markdown
@@ -279,6 +279,10 @@ It may also have some architecture-specific sections. For
example:
* Exception tables.
* Relocations for each of these sections.
+Note that on ARM 32 the sections SHOULD be four byte aligned. Otherwise
+we risk hitting Data Abort exception as un-aligned manipulation of data is
+prohibited on ARM 32.
+
The Xen Live Patch core code loads the payload as a standard ELF binary,
relocates it
and handles the architecture-specifc sections as needed. This process is much
like what the Linux kernel module loader does.
@@ -341,7 +345,7 @@ The size of the structure is 64 bytes on 64-bit
hypervisors. It will be
* `opaque` **MUST** be zero.
The size of the `livepatch_func` array is determined from the ELF section
-size.
+size. On ARM32 this section must by four-byte aligned.
When applying the patch the hypervisor iterates over each `livepatch_func`
structure and the core code inserts a trampoline at `old_addr` to `new_addr`.
diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
index 41378a54ae..d48820f8ba 100644
--- a/xen/arch/arm/arm32/livepatch.c
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -112,6 +112,12 @@ bool arch_livepatch_symbol_deny(const struct livepatch_elf
*elf,
return false;
}
+bool arch_livepatch_verify_alignment(const struct livepatch_elf_sec *sec)
+{
+ /* Unaligned access on ARM 32 will crash with Data Abort. */
+ return (uint32_t)sec->load_addr % sizeof(uint32_t);
+};
+
static s32 get_addend(unsigned char type, void *dest)
{
s32 addend = 0;
@@ -266,6 +272,15 @@ int arch_livepatch_perform(struct livepatch_elf *elf,
elf->name, symndx);
return -EINVAL;
}
+
+ if ( !use_rela )
+ addend = get_addend(type, dest);
+ else if ( (uint32_t)dest % sizeof(uint32_t) )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: dest=%p (%s) is not aligned
properly!\n",
+ elf->name, dest, base->name);
+ return -EINVAL;
+ }
else if ( !elf->sym[symndx].sym )
{
dprintk(XENLOG_ERR, LIVEPATCH "%s: No relative symbol@%u\n",
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index 2247b925a0..7b36210ccd 100644
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -86,6 +86,12 @@ bool arch_livepatch_symbol_deny(const struct livepatch_elf
*elf,
return false;
}
+bool arch_livepatch_verify_alignment(const struct livepatch_elf_sec *sec)
+{
+ /* Unaligned access on ARM 64 is OK. */
+ return false;
+}
+
enum aarch64_reloc_op {
RELOC_OP_NONE,
RELOC_OP_ABS,
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 406eb910cc..b3cbdac9b7 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -148,6 +148,12 @@ bool arch_livepatch_symbol_deny(const struct livepatch_elf
*elf,
return false;
}
+bool arch_livepatch_verify_alignment(const struct livepatch_elf_sec *sec)
+{
+ /* Unaligned access on x86 is fine. */
+ return false;
+}
+
int arch_livepatch_perform_rel(struct livepatch_elf *elf,
const struct livepatch_elf_sec *base,
const struct livepatch_elf_sec *rela)
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 40ff6b3a27..13d8f25a4b 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -472,6 +472,13 @@ static int check_section(const struct livepatch_elf *elf,
return -EINVAL;
}
+ if ( arch_livepatch_verify_alignment(sec) )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: %s is not aligned properly!\n",
+ elf->name, sec->name);
+ return -EINVAL;
+ }
+
return 0;
}
@@ -501,7 +508,12 @@ static int check_special_sections(const struct
livepatch_elf *elf)
elf->name, names[i]);
return -EINVAL;
}
-
+ if ( arch_livepatch_verify_alignment(sec) )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: %s is not aligned properly!\n",
+ elf->name, names[i]);
+ return -EINVAL;
+ }
if ( test_and_set_bit(i, found) )
{
dprintk(XENLOG_ERR, LIVEPATCH "%s: %s was seen more than once!\n",
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 98ec01216b..e9bab87f28 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -76,6 +76,7 @@ void arch_livepatch_init(void);
#include <asm/livepatch.h>
int arch_livepatch_verify_func(const struct livepatch_func *func);
+bool arch_livepatch_verify_alignment(const struct livepatch_elf_sec *sec);
static inline
unsigned int livepatch_insn_len(const struct livepatch_func *func)
{
--
2.13.3
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |