|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen master] libelf: check nul-terminated strings properly
commit 04877847ade4ac9216e9f408fd544ade8f90cf9a
Author: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
AuthorDate: Fri Jun 14 16:39:36 2013 +0100
Commit: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CommitDate: Fri Jun 14 16:39:36 2013 +0100
libelf: check nul-terminated strings properly
It is not safe to simply take pointers into the ELF and use them as C
pointers. They might not be properly nul-terminated (and the pointers
might be wild).
So we are going to introduce a new function elf_strval for safely
getting strings. This will check that the addresses are in range and
that there is a proper nul-terminated string. Of course it might
discover that there isn't. In that case, it will be made to fail.
This means that elf_note_name might fail, too.
For the benefit of call sites which are just going to pass the value
to a printf-like function, we provide elf_strfmt which returns
"(invalid)" on failure rather than NULL.
In this patch we introduce dummy definitions of these functions. We
introduce calls to elf_strval and elf_strfmt everywhere, and update
all the call sites with appropriate error checking.
There is not yet any semantic change, since before this patch all the
places where we introduce elf_strval dereferenced the value anyway, so
it mustn't have been NULL.
In future patches, when elf_strval is made able return NULL, when it
does so it will mark the elf "broken" so that an appropriate
diagnostic can be printed.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Reviewed-by: Chuck Anderson <chuck.anderson@xxxxxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
v7: Change readnotes.c check to use two if statements rather than ||.
v2: Fix coding style, in one "if" statement.
---
tools/xcutils/readnotes.c | 11 ++++++++---
xen/common/libelf/libelf-dominfo.c | 13 ++++++++++---
xen/common/libelf/libelf-tools.c | 10 +++++++---
xen/include/xen/libelf.h | 7 +++++--
4 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c
index 7ff2530..cfae994 100644
--- a/tools/xcutils/readnotes.c
+++ b/tools/xcutils/readnotes.c
@@ -63,7 +63,7 @@ struct setup_header {
static void print_string_note(const char *prefix, struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note)
{
- printf("%s: %s\n", prefix, (char*)elf_note_desc(elf, note));
+ printf("%s: %s\n", prefix, elf_strfmt(elf, elf_note_desc(elf, note)));
}
static void print_numeric_note(const char *prefix, struct elf_binary *elf,
@@ -103,10 +103,14 @@ static int print_notes(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) start,
{
ELF_HANDLE_DECL(elf_note) note;
int notes_found = 0;
+ const char *this_note_name;
for ( note = start; ELF_HANDLE_PTRVAL(note) < ELF_HANDLE_PTRVAL(end);
note = elf_note_next(elf, note) )
{
- if (0 != strcmp(elf_note_name(elf, note), "Xen"))
+ this_note_name = elf_note_name(elf, note);
+ if (NULL == this_note_name)
+ continue;
+ if (0 != strcmp(this_note_name, "Xen"))
continue;
notes_found++;
@@ -294,7 +298,8 @@ int main(int argc, char **argv)
shdr = elf_shdr_by_name(&elf, "__xen_guest");
if (ELF_HANDLE_VALID(shdr))
- printf("__xen_guest: %s\n", (char*)elf_section_start(&elf,
shdr));
+ printf("__xen_guest: %s\n",
+ elf_strfmt(&elf, elf_section_start(&elf, shdr)));
return 0;
}
diff --git a/xen/common/libelf/libelf-dominfo.c
b/xen/common/libelf/libelf-dominfo.c
index 566f6f9..ba0dc83 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -137,7 +137,10 @@ int elf_xen_parse_note(struct elf_binary *elf,
if ( note_desc[type].str )
{
- str = elf_note_desc(elf, note);
+ str = elf_strval(elf, elf_note_desc(elf, note));
+ if (str == NULL)
+ /* elf_strval will mark elf broken if it fails so no need to log */
+ return 0;
elf_msg(elf, "%s: %s = \"%s\"\n", __FUNCTION__,
note_desc[type].name, str);
parms->elf_notes[type].type = XEN_ENT_STR;
@@ -220,6 +223,7 @@ static int elf_xen_parse_notes(struct elf_binary *elf,
{
int xen_elfnotes = 0;
ELF_HANDLE_DECL(elf_note) note;
+ const char *note_name;
parms->elf_note_start = start;
parms->elf_note_end = end;
@@ -227,7 +231,10 @@ static int elf_xen_parse_notes(struct elf_binary *elf,
ELF_HANDLE_PTRVAL(note) < parms->elf_note_end;
note = elf_note_next(elf, note) )
{
- if ( strcmp(elf_note_name(elf, note), "Xen") )
+ note_name = elf_note_name(elf, note);
+ if ( note_name == NULL )
+ continue;
+ if ( strcmp(note_name, "Xen") )
continue;
if ( elf_xen_parse_note(elf, parms, note) )
return -1;
@@ -541,7 +548,7 @@ int elf_xen_parse(struct elf_binary *elf,
parms->elf_note_start = ELF_INVALID_PTRVAL;
parms->elf_note_end = ELF_INVALID_PTRVAL;
elf_msg(elf, "%s: __xen_guest: \"%s\"\n", __FUNCTION__,
- parms->guest_info);
+ elf_strfmt(elf, parms->guest_info));
elf_xen_parse_guest_info(elf, parms);
break;
}
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index bf68bcd..fa7dedd 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -119,7 +119,7 @@ const char *elf_section_name(struct elf_binary *elf,
if ( ELF_PTRVAL_INVALID(elf->sec_strtab) )
return "unknown";
- return elf->sec_strtab + elf_uval(elf, shdr, sh_name);
+ return elf_strval(elf, elf->sec_strtab + elf_uval(elf, shdr, sh_name));
}
ELF_PTRVAL_CONST_VOID elf_section_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr)
@@ -151,6 +151,7 @@ ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary
*elf, const char *sym
ELF_PTRVAL_CONST_VOID end = elf_section_end(elf, elf->sym_tab);
ELF_HANDLE_DECL(elf_sym) sym;
uint64_t info, name;
+ const char *sym_name;
for ( ; ptr < end; ptr += elf_size(elf, sym) )
{
@@ -159,7 +160,10 @@ ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary
*elf, const char *sym
name = elf_uval(elf, sym, st_name);
if ( ELF32_ST_BIND(info) != STB_GLOBAL )
continue;
- if ( strcmp(elf->sym_strtab + name, symbol) )
+ sym_name = elf_strval(elf, elf->sym_strtab + name);
+ if ( sym_name == NULL ) /* out of range, oops */
+ return ELF_INVALID_HANDLE(elf_sym);
+ if ( strcmp(sym_name, symbol) )
continue;
return sym;
}
@@ -177,7 +181,7 @@ ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary
*elf, int index)
const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note)
{
- return ELF_HANDLE_PTRVAL(note) + elf_size(elf, note);
+ return elf_strval(elf, ELF_HANDLE_PTRVAL(note) + elf_size(elf, note));
}
ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note)
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index 7bd3bdb..28c7b11 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -252,6 +252,9 @@ uint64_t elf_access_unsigned(struct elf_binary *elf,
ELF_PTRVAL_CONST_VOID ptr,
uint64_t elf_round_up(struct elf_binary *elf, uint64_t addr);
+#define elf_strval(elf,x) ((const char*)(x)) /* may return NULL in the future
*/
+#define elf_strfmt(elf,x) ((const char*)(x)) /* will return (invalid) instead
*/
+
#define elf_memcpy_safe(elf, dst, src, sz) memcpy((dst),(src),(sz))
#define elf_memset_safe(elf, dst, c, sz) memset((dst),(c),(sz))
/*
@@ -279,7 +282,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct
elf_binary *elf, const char *n
ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, int index);
ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, int index);
-const char *elf_section_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr)
shdr);
+const char *elf_section_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr)
shdr); /* might return NULL if inputs are invalid */
ELF_PTRVAL_CONST_VOID elf_section_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr);
ELF_PTRVAL_CONST_VOID elf_section_end(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr);
@@ -289,7 +292,7 @@ ELF_PTRVAL_CONST_VOID elf_segment_end(struct elf_binary
*elf, ELF_HANDLE_DECL(el
ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary *elf, const char
*symbol);
ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, int index);
-const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note);
+const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note); /* may return NULL */
ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note);
uint64_t elf_note_numeric(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note);
uint64_t elf_note_numeric_array(struct elf_binary *, ELF_HANDLE_DECL(elf_note),
--
generated by git-patchbot for /home/xen/git/xen.git#master
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |