|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v1 3/3] xen/livepatch/ARM32: Don't 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
aligned 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:
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.
However 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 erorr 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.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
xen/arch/arm/arm32/livepatch.c | 18 ++++++++++++++++--
xen/arch/arm/arm64/livepatch.c | 6 ++++++
xen/arch/x86/livepatch.c | 6 ++++++
xen/common/livepatch.c | 37 ++++++++++++++++++++++++++++++++++++-
xen/include/xen/livepatch.h | 1 +
5 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
index 41378a5..ccb9bf8 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;
@@ -233,7 +239,7 @@ int arch_livepatch_perform(struct livepatch_elf *elf,
uint32_t val;
void *dest;
unsigned char type;
- s32 addend;
+ s32 addend = 0;
if ( use_rela )
{
@@ -251,7 +257,6 @@ int arch_livepatch_perform(struct livepatch_elf *elf,
symndx = ELF32_R_SYM(r->r_info);
type = ELF32_R_TYPE(r->r_info);
dest = base->load_addr + r->r_offset; /* P */
- addend = get_addend(type, dest);
}
if ( symndx == STN_UNDEF )
@@ -272,6 +277,15 @@ int arch_livepatch_perform(struct livepatch_elf *elf,
elf->name, symndx);
return -EINVAL;
}
+ 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;
+ }
+
+ if ( !use_rela )
+ addend = get_addend(type, dest);
val = elf->sym[symndx].sym->st_value; /* S */
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index 2247b92..7b36210 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 406eb91..b3cbdac 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 c0eb609..7c52eed 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -495,7 +495,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",
@@ -568,6 +573,12 @@ static int prepare_payload(struct payload *payload,
if ( sec->sec->sh_size % sizeof(*payload->load_funcs) )
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;
+ }
payload->load_funcs = sec->load_addr;
payload->n_load_funcs = sec->sec->sh_size /
sizeof(*payload->load_funcs);
}
@@ -578,6 +589,12 @@ static int prepare_payload(struct payload *payload,
if ( sec->sec->sh_size % sizeof(*payload->unload_funcs) )
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;
+ }
payload->unload_funcs = sec->load_addr;
payload->n_unload_funcs = sec->sec->sh_size /
sizeof(*payload->unload_funcs);
}
@@ -653,6 +670,12 @@ static int prepare_payload(struct payload *payload,
sec->sec->sh_size);
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;
+ }
region->frame[i].bugs = sec->load_addr;
region->frame[i].n_bugs = sec->sec->sh_size /
@@ -671,6 +694,12 @@ static int prepare_payload(struct payload *payload,
elf->name, sizeof(*a));
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;
+ }
start = sec->load_addr;
end = sec->load_addr + sec->sec->sh_size;
@@ -710,6 +739,12 @@ static int prepare_payload(struct payload *payload,
sec->sec->sh_size);
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;
+ }
s = sec->load_addr;
e = sec->load_addr + sec->sec->sh_size;
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index fccff94..290bfd5 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -77,6 +77,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.9.3
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |