Re: [Xen-devel] [PATCH 15/21] libelf: check loops for running away

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Jackson writes ("Re: [Xen-devel] [PATCH 15/21] libelf: check loops for 
running away"):
> George Dunlap writes ("Re: [Xen-devel] [PATCH 15/21] libelf: check loops for 
> running away"):
> > Is there something special about phdr vs shdr?
> 
> You are entirely correct.  I must have misread "continue" as "break".
...
> I will fix these by changing them to use the same checks as elsewhere.

Here's that diff, which I will fold into patch 15 in v8.

Ian.

commit 1c0f8cb92c604d92db9037b5aeadfeadf2197a05
Author: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Date:   Thu Jun 13 18:47:46 2013 +0100

    libelf-dominfo fix two loops

diff --git a/.topmsg b/.topmsg
index eed7c13..405c54f 100644
--- a/.topmsg
+++ b/.topmsg
@@ -32,6 +32,10 @@ This is part of the fix to a security issue, XSA-55.
 Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
 Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
 
+v8: Fix the two loops in libelf-dominfo.c; the comment about
+     PT_NOTE and SHT_NOTE wasn't true because the checks did
+     "continue", not "break".
+
 v5: Fix regression due to wrong image size loop limit calculation.
     Check return value from xc_dom_malloc.
 
diff --git a/xen/common/libelf/libelf-dominfo.c 
b/xen/common/libelf/libelf-dominfo.c
index b0ba4d8..8ca2a33 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -495,13 +495,9 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf,
     for ( i = 0; i < count; i++ )
     {
         phdr = elf_phdr_by_index(elf, i);
-        /*
-         * This test also arranges for the loop to terminate if the
-         * input file has a ridiculous value for the header count: The
-         * first putative header outside the input image will appear
-         * to have type 0 (since out-of-range accesses read as 0) and
-         * PT_NOTE != 0.
-         */
+        if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(phdr), 1) )
+            /* input has an insane program header count field */
+            break;
         if ( elf_uval(elf, phdr, p_type) != PT_NOTE )
             continue;
 
@@ -532,11 +528,10 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf,
         for ( i = 0; i < count; i++ )
         {
             shdr = elf_shdr_by_index(elf, i);
+            if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
+                /* input has an insane section header count field */
+                break;
 
-            /*
-             * See above re guarantee of loop termination.
-             * SHT_NOTE != 0.
-             */
             if ( elf_uval(elf, shdr, sh_type) != SHT_NOTE )
                 continue;
 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel