[PATCH 09/16] libelf: check nul-terminated strings properly

[Ian Jackson <ian.jackson@eu.citrix.com>]

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 <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

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 7ff2530..ca86ba5 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,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++;
@@ -294,7 +297,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 dc9b5ae..0c2c11e 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