From f2c7b1245f48626624599644563f66954fb3be4c Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Fri, 24 May 2013 17:52:42 +0100 Subject: [PATCH 07/14] 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. Signed-off-by: Ian Jackson Acked-by: Ian Campbell Reviewed-by: Konrad Rzeszutek Wilk v2: Fix coding style, in one "if" statement. --- tools/xcutils/readnotes.c | 10 +++++++--- xen/common/libelf/libelf-dominfo.c | 13 ++++++++++--- xen/common/libelf/libelf-tools.c | 10 +++++++--- xen/include/xen/libelf.h | 7 +++++-- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c index af8ea12..9710d58 100644 --- a/tools/xcutils/readnotes.c +++ b/tools/xcutils/readnotes.c @@ -21,7 +21,7 @@ static xc_interface *xch; 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, @@ -61,10 +61,13 @@ 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 || + 0 != strcmp(this_note_name, "Xen")) continue; notes_found++; @@ -217,7 +220,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 1ae57ca..60673cd 100644 --- a/xen/common/libelf/libelf-dominfo.c +++ b/xen/common/libelf/libelf-dominfo.c @@ -133,7 +133,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; @@ -210,6 +213,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; @@ -217,7 +221,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; @@ -525,7 +532,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 af260fa..628c159 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 2063a41..3d8ffa4 100644 --- a/xen/include/xen/libelf.h +++ b/xen/include/xen/libelf.h @@ -262,6 +262,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)) /* @@ -289,7 +292,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); @@ -299,7 +302,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); ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note); -- 1.7.2.5