[Qemu-devel] [PATCH] elf-loader: Avoid calling qsort(NULL, 0, ...) call

[Qemu-devel] [PATCH] elf-loader: Avoid calling qsort(NULL, 0, ...) call

[Philippe Mathieu-Daudé]

This fixes the following ASan warning:

  $ qemu-system-xtensa -M kc705 -m 128M -semihosting -nographic -monitor null -kernel Image.elf
  include/hw/elf_ops.h:179:5: runtime error: null pointer passed as argument 1, which is declared to never be null

Reported-by: AddressSanitizer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/elf_ops.h | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index b6e19e35d0..f0ac7c6c4e 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -110,7 +110,7 @@ static int glue(load_symbols, SZ)(struct elfhdr *ehdr, int fd, int must_swab,
     struct elf_shdr *symtab, *strtab, *shdr_table = NULL;
     struct elf_sym *syms = NULL;
     struct syminfo *s;
-    int nsyms, i;
+    int nsyms, i, ret = -1;
     char *str = NULL;
 
     shdr_table = load_at(fd, ehdr->e_shoff,
@@ -143,6 +143,7 @@ static int glue(load_symbols, SZ)(struct elfhdr *ehdr, int fd, int must_swab,
     if (!str) {
         goto fail;
     }
+    ret = 0;
 
     i = 0;
     while (i < nsyms) {
@@ -170,30 +171,34 @@ static int glue(load_symbols, SZ)(struct elfhdr *ehdr, int fd, int must_swab,
         }
         i++;
     }
-    syms = g_realloc(syms, nsyms * sizeof(*syms));
+    if (nsyms) {
+        syms = g_realloc(syms, nsyms * sizeof(*syms));
 
-    qsort(syms, nsyms, sizeof(*syms), glue(symcmp, SZ));
-    for (i = 0; i < nsyms - 1; i++) {
-        if (syms[i].st_size == 0) {
-            syms[i].st_size = syms[i + 1].st_value - syms[i].st_value;
+        qsort(syms, nsyms, sizeof(*syms), glue(symcmp, SZ));
+        for (i = 0; i < nsyms - 1; i++) {
+            if (syms[i].st_size == 0) {
+                syms[i].st_size = syms[i + 1].st_value - syms[i].st_value;
+            }
         }
+
+        /* Commit */
+        s = g_malloc0(sizeof(*s));
+        s->lookup_symbol = glue(lookup_symbol, SZ);
+        glue(s->disas_symtab.elf, SZ) = syms;
+        s->disas_num_syms = nsyms;
+        s->disas_strtab = str;
+        s->next = syminfos;
+        syminfos = s;
+
+        goto out;
     }
 
-    /* Commit */
-    s = g_malloc0(sizeof(*s));
-    s->lookup_symbol = glue(lookup_symbol, SZ);
-    glue(s->disas_symtab.elf, SZ) = syms;
-    s->disas_num_syms = nsyms;
-    s->disas_strtab = str;
-    s->next = syminfos;
-    syminfos = s;
-    g_free(shdr_table);
-    return 0;
  fail:
     g_free(syms);
     g_free(str);
+ out:
     g_free(shdr_table);
-    return -1;
+    return ret;
 }
 
 static int glue(elf_reloc, SZ)(struct elfhdr *ehdr, int fd, int must_swab,
-- 
2.17.0

Re: [Qemu-devel] [PATCH] elf-loader: Avoid calling qsort(NULL, 0, ...) call

[Richard Henderson]

On 04/21/2018 01:21 PM, Philippe Mathieu-Daudé wrote:
> This fixes the following ASan warning:
> 
>   $ qemu-system-xtensa -M kc705 -m 128M -semihosting -nographic -monitor null -kernel Image.elf
>   include/hw/elf_ops.h:179:5: runtime error: null pointer passed as argument 1, which is declared to never be null
> 
> Reported-by: AddressSanitizer
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/elf_ops.h | 39 ++++++++++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 17 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH] elf-loader: Avoid calling qsort(NULL, 0, ...) call

[Philippe Mathieu-Daudé]

Hi Paolo,

On 04/22/2018 04:22 PM, Richard Henderson wrote:
> On 04/21/2018 01:21 PM, Philippe Mathieu-Daudé wrote:
>> This fixes the following ASan warning:
>>
>>   $ qemu-system-xtensa -M kc705 -m 128M -semihosting -nographic -monitor null -kernel Image.elf
>>   include/hw/elf_ops.h:179:5: runtime error: null pointer passed as argument 1, which is declared to never be null
>>
>> Reported-by: AddressSanitizer
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  include/hw/elf_ops.h | 39 ++++++++++++++++++++++-----------------
>>  1 file changed, 22 insertions(+), 17 deletions(-)
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Since this patch isn't Xtensa specific, can it goes via your MISC tree?

Thanks,

Phil.