From 59f66d58180832af6b99a9e4489031b5c2f627ab Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Fri, 14 Jun 2013 16:43:17 +0100 Subject: [PATCH 09/23] tools/xcutils/readnotes: adjust print_l1_mfn_valid_note Use the new PTRVAL macros and elf_access_unsigned in print_l1_mfn_valid_note. No functional change unless the input is wrong, or we are reading a file for a different endianness. Separated out from the previous patch because this change does produce a difference in the generated code. This is part of the fix to a security issue, XSA-55. Signed-off-by: Ian Jackson Acked-by: Ian Campbell --- tools/xcutils/readnotes.c | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c index 2af047d..7ff2530 100644 --- a/tools/xcutils/readnotes.c +++ b/tools/xcutils/readnotes.c @@ -77,22 +77,23 @@ static void print_numeric_note(const char *prefix, struct elf_binary *elf, } static void print_l1_mfn_valid_note(const char *prefix, struct elf_binary *elf, - const elf_note *note) + ELF_HANDLE_DECL(elf_note) note) { int descsz = elf_uval(elf, note, descsz); - const uint32_t *desc32 = elf_note_desc(elf, note); - const uint64_t *desc64 = elf_note_desc(elf, note); + ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note); /* XXX should be able to cope with a list of values. */ switch ( descsz / 2 ) { case 8: printf("%s: mask=%#"PRIx64" value=%#"PRIx64"\n", prefix, - desc64[0], desc64[1]); + elf_access_unsigned(elf, desc, 0, 8), + elf_access_unsigned(elf, desc, 8, 8)); break; case 4: printf("%s: mask=%#"PRIx32" value=%#"PRIx32"\n", prefix, - desc32[0],desc32[1]); + (uint32_t)elf_access_unsigned(elf, desc, 0, 4), + (uint32_t)elf_access_unsigned(elf, desc, 4, 4)); break; } -- 1.7.2.5 #From db14d5bd9b6508adfcd2b910f454fae12fa4ba00 Mon Sep 17 00:00:00 2001 #From: Ian Jackson #Date: Fri, 14 Jun 2013 16:43:17 +0100 #Subject: [PATCH 10/23] 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 #Acked-by: Ian Campbell #Reviewed-by: Konrad Rzeszutek Wilk #--- # 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 7140d59..b217f8f 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 f1fd886..3a0cde1 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 cefd3d3..af5b5c5 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), -- 1.7.2.5