|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2] xen/livepatch: Clean up arch relocation handling
On Tue, Jun 13, 2017 at 09:51:35PM +0100, Andrew Cooper wrote:
> * Reduce symbol scope and initalisation as much as possible
> * Annotate a fallthrough case in arm64
> * Fix switch statement style in arm32
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
>
> The purpose of this patch is simply to make the following patch easier to
> review.
> ---
> xen/arch/arm/arm32/livepatch.c | 27 ++++++++++++---------------
> xen/arch/arm/arm64/livepatch.c | 19 +++++++------------
> xen/arch/x86/livepatch.c | 13 +++++--------
> 3 files changed, 24 insertions(+), 35 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
> index a7fd5e2..a328179 100644
> --- a/xen/arch/arm/arm32/livepatch.c
> +++ b/xen/arch/arm/arm32/livepatch.c
> @@ -224,21 +224,21 @@ int arch_livepatch_perform(struct livepatch_elf *elf,
> const struct livepatch_elf_sec *rela,
> bool use_rela)
> {
> - const Elf_RelA *r_a;
> - const Elf_Rel *r;
> - unsigned int symndx, i;
> - uint32_t val;
> - void *dest;
> + unsigned int i;
> int rc = 0;
>
> for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ )
> {
> + unsigned int symndx;
> + uint32_t val;
> + void *dest;
> unsigned char type;
> - s32 addend = 0;
> + s32 addend;
>
> if ( use_rela )
> {
> - r_a = rela->data + i * rela->sec->sh_entsize;
> + const Elf_RelA *r_a = rela->data + i * rela->sec->sh_entsize;
> +
> symndx = ELF32_R_SYM(r_a->r_info);
> type = ELF32_R_TYPE(r_a->r_info);
> dest = base->load_addr + r_a->r_offset; /* P */
> @@ -246,10 +246,12 @@ int arch_livepatch_perform(struct livepatch_elf *elf,
> }
> else
> {
> - r = rela->data + i * rela->sec->sh_entsize;
> + const Elf_Rel *r = rela->data + i * rela->sec->sh_entsize;
> +
> 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 > elf->nsym )
> @@ -259,13 +261,11 @@ int arch_livepatch_perform(struct livepatch_elf *elf,
> return -EINVAL;
> }
>
> - if ( !use_rela )
> - addend = get_addend(type, dest);
This was added right after the symndx > elf->nsym check as
way to make sure we won't dereference the dest (b/c the symbol
may be outside the bounds).
> -
> val = elf->sym[symndx].sym->st_value; /* S */
>
> rc = perform_rel(type, dest, val, addend);
> - switch ( rc ) {
> + switch ( rc )
> + {
> case -EOVERFLOW:
> dprintk(XENLOG_ERR, LIVEPATCH "%s: Overflow in relocation %u in
> %s for %s!\n",
> elf->name, i, rela->name, base->name);
> @@ -275,9 +275,6 @@ int arch_livepatch_perform(struct livepatch_elf *elf,
> dprintk(XENLOG_ERR, LIVEPATCH "%s: Unhandled relocation #%x\n",
> elf->name, type);
> break;
> -
> - default:
> - break;
> }
>
> if ( rc )
> diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
> index dae64f5..63929b1 100644
> --- a/xen/arch/arm/arm64/livepatch.c
> +++ b/xen/arch/arm/arm64/livepatch.c
> @@ -241,19 +241,16 @@ int arch_livepatch_perform_rela(struct livepatch_elf
> *elf,
> const struct livepatch_elf_sec *base,
> const struct livepatch_elf_sec *rela)
> {
> - const Elf_RelA *r;
> - unsigned int symndx, i;
> - uint64_t val;
> - void *dest;
> - bool_t overflow_check;
> + unsigned int i;
>
> for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ )
> {
> + const Elf_RelA *r = rela->data + i * rela->sec->sh_entsize;
> + unsigned int symndx = ELF64_R_SYM(r->r_info);
> + void *dest = base->load_addr + r->r_offset; /* P */
> + bool overflow_check = true;
> int ovf = 0;
> -
> - r = rela->data + i * rela->sec->sh_entsize;
> -
> - symndx = ELF64_R_SYM(r->r_info);
> + uint64_t val;
>
> if ( symndx > elf->nsym )
> {
> @@ -262,11 +259,8 @@ int arch_livepatch_perform_rela(struct livepatch_elf
> *elf,
> return -EINVAL;
> }
>
> - dest = base->load_addr + r->r_offset; /* P */
> val = elf->sym[symndx].sym->st_value + r->r_addend; /* S+A */
>
> - overflow_check = true;
> -
> /* ARM64 operations at minimum are always 32-bit. */
> if ( r->r_offset >= base->sec->sh_size ||
> (r->r_offset + sizeof(uint32_t)) > base->sec->sh_size )
> @@ -403,6 +397,7 @@ int arch_livepatch_perform_rela(struct livepatch_elf *elf,
>
> case R_AARCH64_ADR_PREL_PG_HI21_NC:
> overflow_check = false;
> + /* Fallthrough. */
> case R_AARCH64_ADR_PREL_PG_HI21:
> ovf = reloc_insn_imm(RELOC_OP_PAGE, dest, val, 12, 21,
> AARCH64_INSN_IMM_ADR);
OK, that part is fine.
> diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
> index dd50dd1..7917610 100644
> --- a/xen/arch/x86/livepatch.c
> +++ b/xen/arch/x86/livepatch.c
> @@ -161,16 +161,14 @@ int arch_livepatch_perform_rela(struct livepatch_elf
> *elf,
> const struct livepatch_elf_sec *base,
> const struct livepatch_elf_sec *rela)
> {
> - const Elf_RelA *r;
> - unsigned int symndx, i;
> - uint64_t val;
> - uint8_t *dest;
> + unsigned int i;
>
> for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ )
> {
> - r = rela->data + i * rela->sec->sh_entsize;
> -
> - symndx = ELF64_R_SYM(r->r_info);
> + const Elf_RelA *r = rela->data + i * rela->sec->sh_entsize;
> + unsigned int symndx = ELF64_R_SYM(r->r_info);
> + uint8_t *dest = base->load_addr + r->r_offset;
> + uint64_t val;
>
> if ( symndx > elf->nsym )
> {
> @@ -179,7 +177,6 @@ int arch_livepatch_perform_rela(struct livepatch_elf *elf,
> return -EINVAL;
> }
>
> - dest = base->load_addr + r->r_offset;
> val = r->r_addend + elf->sym[symndx].sym->st_value;
>
> switch ( ELF64_R_TYPE(r->r_info) )
And this is fine too.
> --
> 2.1.4
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |