|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH v3 1/5] xen/livepatch: drop load_addr Elf section field
The Elf loading logic will initially use the `data` section field to stash a
pointer to the temporary loaded data (from the buffer allocated in
livepatch_upload(), which is later relocated and the new pointer stashed in
`load_addr`.
Remove this dual field usage and use an `addr` uniformly. Initially data will
point to the temporary buffer, until relocation happens, at which point the
pointer will be updated to the relocated address.
This avoids leaking a dangling pointer in the `data` field once the temporary
buffer is freed by livepatch_upload().
Note the `addr` field cannot retain the const attribute from the previous
`data`field, as there's logic that performs manipulations against the loaded
sections, like applying relocations or sorting the exception table.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
Changes since v2:
- New in this version.
---
xen/arch/arm/arm32/livepatch.c | 8 +++---
xen/arch/arm/arm64/livepatch.c | 4 +--
xen/arch/x86/livepatch.c | 4 +--
xen/common/livepatch.c | 43 ++++++++++++++++++---------------
xen/common/livepatch_elf.c | 20 +++++++--------
xen/include/xen/livepatch_elf.h | 11 +++++----
6 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
index d50066564666..134d07a175bb 100644
--- a/xen/arch/arm/arm32/livepatch.c
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -239,20 +239,20 @@ int arch_livepatch_perform(struct livepatch_elf *elf,
if ( use_rela )
{
- const Elf_RelA *r_a = rela->data + i * rela->sec->sh_entsize;
+ const Elf_RelA *r_a = rela->addr + 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 */
+ dest = base->addr + r_a->r_offset; /* P */
addend = r_a->r_addend;
}
else
{
- const Elf_Rel *r = rela->data + i * rela->sec->sh_entsize;
+ const Elf_Rel *r = rela->addr + 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 */
+ dest = base->addr + r->r_offset; /* P */
addend = get_addend(type, dest);
}
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index dfb72be44fb8..d80051f9dc67 100644
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -246,9 +246,9 @@ int arch_livepatch_perform_rela(struct livepatch_elf *elf,
for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ )
{
- const Elf_RelA *r = rela->data + i * rela->sec->sh_entsize;
+ const Elf_RelA *r = rela->addr + i * rela->sec->sh_entsize;
unsigned int symndx = ELF64_R_SYM(r->r_info);
- void *dest = base->load_addr + r->r_offset; /* P */
+ void *dest = base->addr + r->r_offset; /* P */
bool overflow_check = true;
int ovf = 0;
uint64_t val;
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 4f76127e1f77..be40f625d206 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -258,9 +258,9 @@ int arch_livepatch_perform_rela(struct livepatch_elf *elf,
for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ )
{
- const Elf_RelA *r = rela->data + i * rela->sec->sh_entsize;
+ const Elf_RelA *r = rela->addr + i * rela->sec->sh_entsize;
unsigned int symndx = ELF64_R_SYM(r->r_info);
- uint8_t *dest = base->load_addr + r->r_offset;
+ uint8_t *dest = base->addr + r->r_offset;
uint64_t val;
if ( symndx == STN_UNDEF )
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index df41dcce970a..7e6bf58f4408 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -371,18 +371,21 @@ static int move_payload(struct payload *payload, struct
livepatch_elf *elf)
ASSERT(offset[i] != UINT_MAX);
- elf->sec[i].load_addr = buf + offset[i];
+ buf += offset[i];
/* Don't copy NOBITS - such as BSS. */
if ( elf->sec[i].sec->sh_type != SHT_NOBITS )
{
- memcpy(elf->sec[i].load_addr, elf->sec[i].data,
+ memcpy(buf, elf->sec[i].addr,
elf->sec[i].sec->sh_size);
dprintk(XENLOG_DEBUG, LIVEPATCH "%s: Loaded %s at %p\n",
- elf->name, elf->sec[i].name, elf->sec[i].load_addr);
+ elf->name, elf->sec[i].name, buf);
}
else
- memset(elf->sec[i].load_addr, 0, elf->sec[i].sec->sh_size);
+ memset(buf, 0, elf->sec[i].sec->sh_size);
+
+ /* Replace the temporary buffer with the relocated one. */
+ elf->sec[i].addr = buf;
}
}
@@ -616,7 +619,7 @@ static inline int livepatch_check_expectations(const struct
payload *payload)
break;
\
if ( !section_ok(elf, __sec, sizeof(*hook)) || __sec->sec->sh_size !=
sizeof(*hook) ) \
return -EINVAL;
\
- hook = __sec->load_addr;
\
+ hook = __sec->addr;
\
} while (0)
/*
@@ -630,7 +633,7 @@ static inline int livepatch_check_expectations(const struct
payload *payload)
break;
\
if ( !section_ok(elf, __sec, sizeof(*hook)) )
\
return -EINVAL;
\
- hook = __sec->load_addr;
\
+ hook = __sec->addr;
\
nhooks = __sec->sec->sh_size / sizeof(*hook);
\
} while (0)
@@ -650,7 +653,7 @@ static int prepare_payload(struct payload *payload,
if ( !section_ok(elf, sec, sizeof(*payload->funcs)) )
return -EINVAL;
- payload->funcs = funcs = sec->load_addr;
+ payload->funcs = funcs = sec->addr;
payload->nfuncs = sec->sec->sh_size / sizeof(*payload->funcs);
payload->fstate = xzalloc_array(typeof(*payload->fstate),
@@ -709,7 +712,7 @@ static int prepare_payload(struct payload *payload,
{
const struct payload *data;
- n = sec->load_addr;
+ n = sec->addr;
if ( sec->sec->sh_size <= sizeof(*n) )
return -EINVAL;
@@ -739,7 +742,7 @@ static int prepare_payload(struct payload *payload,
sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_DEPENDS);
if ( sec )
{
- n = sec->load_addr;
+ n = sec->addr;
if ( sec->sec->sh_size <= sizeof(*n) )
return -EINVAL;
@@ -755,7 +758,7 @@ static int prepare_payload(struct payload *payload,
sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_XEN_DEPENDS);
if ( sec )
{
- n = sec->load_addr;
+ n = sec->addr;
if ( sec->sec->sh_size <= sizeof(*n) )
return -EINVAL;
@@ -794,8 +797,8 @@ static int prepare_payload(struct payload *payload,
if ( !section_ok(elf, sec, sizeof(*region->frame[i].start)) )
return -EINVAL;
- region->frame[i].start = sec->load_addr;
- region->frame[i].stop = sec->load_addr + sec->sec->sh_size;
+ region->frame[i].start = sec->addr;
+ region->frame[i].stop = sec->addr + sec->sec->sh_size;
}
sec = livepatch_elf_sec_by_name(elf, ".altinstructions");
@@ -843,8 +846,8 @@ static int prepare_payload(struct payload *payload,
return -EINVAL;
}
- start = sec->load_addr;
- end = sec->load_addr + sec->sec->sh_size;
+ start = sec->addr;
+ end = sec->addr + sec->sec->sh_size;
for ( a = start; a < end; a++ )
{
@@ -867,14 +870,14 @@ static int prepare_payload(struct payload *payload,
* repl must be fully within .altinstr_replacement, even if the
* replacement and the section happen to both have zero length.
*/
- if ( repl < repl_sec->load_addr ||
+ if ( repl < repl_sec->addr ||
a->repl_len > repl_sec->sec->sh_size ||
- repl + a->repl_len > repl_sec->load_addr +
repl_sec->sec->sh_size )
+ repl + a->repl_len > repl_sec->addr + repl_sec->sec->sh_size )
{
printk(XENLOG_ERR LIVEPATCH
"%s Alternative repl %p+%#x outside
.altinstr_replacement %p+%#"PRIxElfWord"\n",
elf->name, repl, a->repl_len,
- repl_sec->load_addr, repl_sec->sec->sh_size);
+ repl_sec->addr, repl_sec->sec->sh_size);
return -EINVAL;
}
}
@@ -896,8 +899,8 @@ static int prepare_payload(struct payload *payload,
if ( !section_ok(elf, sec, sizeof(*region->ex)) )
return -EINVAL;
- s = sec->load_addr;
- e = sec->load_addr + sec->sec->sh_size;
+ s = sec->addr;
+ e = sec->addr + sec->sec->sh_size;
sort_exception_table(s ,e);
@@ -916,7 +919,7 @@ static int prepare_payload(struct payload *payload,
if ( !section_ok(elf, sec, sizeof(*payload->metadata.data)) )
return -EINVAL;
- payload->metadata.data = sec->load_addr;
+ payload->metadata.data = sec->addr;
payload->metadata.len = sec->sec->sh_size;
/* The metadata is required to consists of null terminated strings. */
diff --git a/xen/common/livepatch_elf.c b/xen/common/livepatch_elf.c
index 45d73912a3cd..25ce1bd5a0ad 100644
--- a/xen/common/livepatch_elf.c
+++ b/xen/common/livepatch_elf.c
@@ -36,7 +36,7 @@ static int elf_verify_strtab(const struct livepatch_elf_sec
*sec)
if ( !s->sh_size )
return -EINVAL;
- contents = sec->data;
+ contents = sec->addr;
if ( contents[0] || contents[s->sh_size - 1] )
return -EINVAL;
@@ -44,7 +44,7 @@ static int elf_verify_strtab(const struct livepatch_elf_sec
*sec)
return 0;
}
-static int elf_resolve_sections(struct livepatch_elf *elf, const void *data)
+static int elf_resolve_sections(struct livepatch_elf *elf, void *data)
{
struct livepatch_elf_sec *sec;
unsigned int i;
@@ -104,7 +104,7 @@ static int elf_resolve_sections(struct livepatch_elf *elf,
const void *data)
sec[i].sec->sh_size > LIVEPATCH_MAX_SIZE )
return -EINVAL;
- sec[i].data = data + delta;
+ sec[i].addr = data + delta;
/* Name is populated in elf_resolve_section_names. */
sec[i].name = NULL;
@@ -226,14 +226,14 @@ static int elf_get_sym(struct livepatch_elf *elf, const
void *data)
strtab_sec = elf->strtab;
/* Pointers arithmetic to get file offset. */
- offset = strtab_sec->data - data;
+ offset = strtab_sec->addr - data;
/* Checked already in elf_resolve_sections, but just in case. */
ASSERT(offset == strtab_sec->sec->sh_offset);
ASSERT(offset < elf->len && (offset + strtab_sec->sec->sh_size <=
elf->len));
- /* symtab_sec->data was computed in elf_resolve_sections. */
- ASSERT((symtab_sec->sec->sh_offset + data) == symtab_sec->data);
+ /* symtab_sec->addr was computed in elf_resolve_sections. */
+ ASSERT((symtab_sec->sec->sh_offset + data) == symtab_sec->addr);
/* No need to check values as elf_resolve_sections did it. */
nsym = symtab_sec->sec->sh_size / symtab_sec->sec->sh_entsize;
@@ -251,7 +251,7 @@ static int elf_get_sym(struct livepatch_elf *elf, const
void *data)
for ( i = 1; i < nsym; i++ )
{
- const Elf_Sym *s = symtab_sec->data + symtab_sec->sec->sh_entsize * i;
+ const Elf_Sym *s = symtab_sec->addr + symtab_sec->sec->sh_entsize * i;
delta = s->st_name;
/* Boundary check within the .strtab. */
@@ -263,7 +263,7 @@ static int elf_get_sym(struct livepatch_elf *elf, const
void *data)
}
sym[i].sym = s;
- sym[i].name = strtab_sec->data + delta;
+ sym[i].name = strtab_sec->addr + delta;
if ( arch_livepatch_symbol_deny(elf, &sym[i]) )
{
printk(XENLOG_ERR LIVEPATCH "%s: Symbol '%s' should not be in
payload\n",
@@ -342,7 +342,7 @@ int livepatch_elf_resolve_symbols(struct livepatch_elf *elf)
break;
}
- st_value += (unsigned long)elf->sec[idx].load_addr;
+ st_value += (unsigned long)elf->sec[idx].addr;
if ( elf->sym[i].name )
dprintk(XENLOG_DEBUG, LIVEPATCH "%s: Symbol resolved: %s =>
%#"PRIxElfAddr" (%s)\n",
elf->name, elf->sym[i].name,
@@ -503,7 +503,7 @@ static int livepatch_header_check(const struct
livepatch_elf *elf)
return 0;
}
-int livepatch_elf_load(struct livepatch_elf *elf, const void *data)
+int livepatch_elf_load(struct livepatch_elf *elf, void *data)
{
int rc;
diff --git a/xen/include/xen/livepatch_elf.h b/xen/include/xen/livepatch_elf.h
index 7116deaddc28..842111e14518 100644
--- a/xen/include/xen/livepatch_elf.h
+++ b/xen/include/xen/livepatch_elf.h
@@ -13,10 +13,11 @@ struct livepatch_elf_sec {
const Elf_Shdr *sec; /* Hooked up in
elf_resolve_sections.*/
const char *name; /* Human readable name hooked in
elf_resolve_section_names. */
- const void *data; /* Pointer to the section (done by
- elf_resolve_sections). */
- void *load_addr; /* A pointer to the allocated
destination.
- Done by load_payload_data. */
+ void *addr; /*
+ * Pointer to the section. This is
+ * first a temporary buffer, then
+ * later the relocated load address.
+ */
};
struct livepatch_elf_sym {
@@ -41,7 +42,7 @@ struct livepatch_elf {
const struct livepatch_elf_sec *
livepatch_elf_sec_by_name(const struct livepatch_elf *elf,
const char *name);
-int livepatch_elf_load(struct livepatch_elf *elf, const void *data);
+int livepatch_elf_load(struct livepatch_elf *elf, void *data);
void livepatch_elf_free(struct livepatch_elf *elf);
int livepatch_elf_resolve_symbols(struct livepatch_elf *elf);
--
2.46.0
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |