PR/134: Don't seek all overr the place when reading elf files, homogenize

the seeking to seek to the proper offset for each header structure reading
instead of saving and restoring offsets in random places.

diff --git a/ChangeLog b/ChangeLog
index 49325356e6823e8ede61249f31461fdf407e8c08..a8d9154d9e5d753ee56ce1a02ddf1b4f01486f53 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2011-08-17  14:32  Christos Zoulas <christos@zoulas.com>
+
+       * Fix ELF lseek(2) madness. Inspired by PR/134 by Jan Kaluza
+
 2011-07-12  12:32  Reuben Thomas <rrt@sc3d.org>
 
        * Fix detection of Zip files (Mantis #128).

diff --git a/src/elfclass.h b/src/elfclass.h
index 7f3da864a596c53d5c58961ae9558b762bd6849c..2e7741b3fc00a051f4ec145918b1a5141044bd83 100644 (file)
--- a/src/elfclass.h
+++ b/src/elfclass.h
@@ -59,8 +59,7 @@
                    (off_t)elf_getu(swap, elfhdr.e_shoff),
                    elf_getu16(swap, elfhdr.e_shnum),
                    (size_t)elf_getu16(swap, elfhdr.e_shentsize),
-                   &flags,
-                   elf_getu16(swap, elfhdr.e_machine)) == -1)
+                   fsize, &flags, elf_getu16(swap, elfhdr.e_machine)) == -1)
                        return -1;
                break;
 

diff --git a/src/readelf.c b/src/readelf.c
index 131bd556b0114e2bc11a5073fee22bf5d26e33e8..d005285642f431497b8ff7e27ab70030d0178d59 100644 (file)
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -27,7 +27,7 @@
 #include "file.h"
 
 #ifndef lint
-FILE_RCSID("@(#)$File: readelf.c,v 1.87 2011/05/13 22:15:24 christos Exp $")
+FILE_RCSID("@(#)$File: readelf.c,v 1.88 2011/07/19 18:54:25 christos Exp $")
 #endif
 
 #ifdef BUILTIN_ELF
@@ -47,8 +47,8 @@ private int dophn_core(struct magic_set *, int, int, int, off_t, int, size_t,
 #endif
 private int dophn_exec(struct magic_set *, int, int, int, off_t, int, size_t,
     off_t, int *, int);
-private int doshn(struct magic_set *, int, int, int, off_t, int, size_t, int *,
-    int);
+private int doshn(struct magic_set *, int, int, int, off_t, int, size_t,
+    off_t, int *, int);
 private size_t donote(struct magic_set *, void *, size_t, size_t, int,
     int, size_t, int *);
 
@@ -144,7 +144,7 @@ getu64(int swap, uint64_t value)
 #define xsh_size       (clazz == ELFCLASS32                    \
                         ? elf_getu32(swap, sh32.sh_size)       \
                         : elf_getu64(swap, sh64.sh_size))
-#define xsh_offset     (clazz == ELFCLASS32                    \
+#define xsh_offset     (off_t)(clazz == ELFCLASS32             \
                         ? elf_getu32(swap, sh32.sh_offset)     \
                         : elf_getu64(swap, sh64.sh_offset))
 #define xsh_type       (clazz == ELFCLASS32                    \
@@ -298,13 +298,6 @@ dophn_core(struct magic_set *ms, int clazz, int swap, int fd, off_t off,
        size_t offset;
        unsigned char nbuf[BUFSIZ];
        ssize_t bufsize;
-       off_t savedoffset;
-       struct stat st;
-
-       if (fstat(fd, &st) < 0) {
-               file_badread(ms);
-               return -1;
-       }
 
        if (size != xph_sizeof) {
                if (file_printf(ms, ", corrupted program header size") == -1)
@@ -316,7 +309,7 @@ dophn_core(struct magic_set *ms, int clazz, int swap, int fd, off_t off,
         * Loop through all the program headers.
         */
        for ( ; num; num--) {
-               if ((savedoffset = lseek(fd, off, SEEK_SET)) == (off_t)-1) {
+               if (lseek(fd, off, SEEK_SET) == (off_t)-1) {
                        file_badseek(ms);
                        return -1;
                }
@@ -324,15 +317,13 @@ dophn_core(struct magic_set *ms, int clazz, int swap, int fd, off_t off,
                        file_badread(ms);
                        return -1;
                }
+               off += size;
+
                if (xph_offset > fsize) {
-                       if (lseek(fd, savedoffset, SEEK_SET) == (off_t)-1) {
-                               file_badseek(ms);
-                               return -1;
-                       }
+                       /* Perhaps warn here */
                        continue;
                }
 
-               off += size;
                if (xph_type != PT_NOTE)
                        continue;
 
@@ -851,7 +842,7 @@ static const cap_desc_t cap_desc_386[] = {
 
 private int
 doshn(struct magic_set *ms, int clazz, int swap, int fd, off_t off, int num,
-    size_t size, int *flags, int mach)
+    size_t size, off_t fsize, int *flags, int mach)
 {
        Elf32_Shdr sh32;
        Elf64_Shdr sh64;
@@ -867,16 +858,22 @@ doshn(struct magic_set *ms, int clazz, int swap, int fd, off_t off, int num,
                return 0;
        }
 
-       if (lseek(fd, off, SEEK_SET) == (off_t)-1) {
-               file_badseek(ms);
-               return -1;
-       }
-
        for ( ; num; num--) {
+               if (lseek(fd, off, SEEK_SET) == (off_t)-1) {
+                       file_badseek(ms);
+                       return -1;
+               }
                if (read(fd, xsh_addr, xsh_sizeof) == -1) {
                        file_badread(ms);
                        return -1;
                }
+               off += size;
+
+               if (xsh_offset > fsize) {
+                       /* Perhaps warn here */
+                       continue;
+               }
+
                switch (xsh_type) {
                case SHT_SYMTAB:
 #if 0
@@ -885,11 +882,6 @@ doshn(struct magic_set *ms, int clazz, int swap, int fd, off_t off, int num,
                        stripped = 0;
                        break;
                case SHT_NOTE:
-                       if ((off = lseek(fd, (off_t)0, SEEK_CUR)) ==
-                           (off_t)-1) {
-                               file_badread(ms);
-                               return -1;
-                       }
                        if ((nbuf = malloc((size_t)xsh_size)) == NULL) {
                                file_error(ms, errno, "Cannot allocate memory"
                                    " for note");
@@ -918,24 +910,14 @@ doshn(struct magic_set *ms, int clazz, int swap, int fd, off_t off, int num,
                                if (noff == 0)
                                        break;
                        }
-                       if ((lseek(fd, off, SEEK_SET)) == (off_t)-1) {
-                               free(nbuf);
-                               file_badread(ms);
-                               return -1;
-                       }
                        free(nbuf);
                        break;
                case SHT_SUNW_cap:
                    {
                        off_t coff;
-                       if ((off = lseek(fd, (off_t)0, SEEK_CUR)) ==
-                           (off_t)-1) {
-                               file_badread(ms);
-                               return -1;
-                       }
                        if (lseek(fd, (off_t)xsh_offset, SEEK_SET) ==
                            (off_t)-1) {
-                               file_badread(ms);
+                               file_badseek(ms);
                                return -1;
                        }
                        coff = 0;
@@ -972,10 +954,6 @@ doshn(struct magic_set *ms, int clazz, int swap, int fd, off_t off, int num,
                                        break;
                                }
                        }
-                       if (lseek(fd, off, SEEK_SET) == (off_t)-1) {
-                               file_badread(ms);
-                               return -1;
-                       }
                        break;
                    }
                }
@@ -1059,13 +1037,6 @@ dophn_exec(struct magic_set *ms, int clazz, int swap, int fd, off_t off,
        unsigned char nbuf[BUFSIZ];
        ssize_t bufsize;
        size_t offset, align;
-       off_t savedoffset = (off_t)-1;
-       struct stat st;
-
-       if (fstat(fd, &st) < 0) {
-               file_badread(ms);
-               return -1;
-       }
        
        if (size != xph_sizeof) {
                if (file_printf(ms, ", corrupted program header size") == -1)
@@ -1073,34 +1044,20 @@ dophn_exec(struct magic_set *ms, int clazz, int swap, int fd, off_t off,
                return 0;
        }
 
-       if (lseek(fd, off, SEEK_SET) == (off_t)-1) {
-               file_badseek(ms);
-               return -1;
-       }
-
        for ( ; num; num--) {
-               if (read(fd, xph_addr, xph_sizeof) == -1) {
-                       file_badread(ms);
+               if (lseek(fd, off, SEEK_SET) == (off_t)-1) {
+                       file_badseek(ms);
                        return -1;
                }
-               if (xph_offset > st.st_size && savedoffset != (off_t)-1) {
-                       if (lseek(fd, savedoffset, SEEK_SET) == (off_t)-1) {
-                               file_badseek(ms);
-                               return -1;
-                       }
-                       continue;
-               }
 
-               if ((savedoffset = lseek(fd, (off_t)0, SEEK_CUR)) == (off_t)-1) {
-                       file_badseek(ms);
+               if (read(fd, xph_addr, xph_sizeof) == -1) {
+                       file_badread(ms);
                        return -1;
                }
 
+               off += size;
                if (xph_offset > fsize) {
-                       if (lseek(fd, savedoffset, SEEK_SET) == (off_t)-1) {
-                               file_badseek(ms);
-                               return -1;
-                       }
+                       /* Maybe warn here? */
                        continue;
                }
 
@@ -1125,8 +1082,7 @@ dophn_exec(struct magic_set *ms, int clazz, int swap, int fd, off_t off,
                         * This is a PT_NOTE section; loop through all the notes
                         * in the section.
                         */
-                       if (lseek(fd, xph_offset, SEEK_SET)
-                           == (off_t)-1) {
+                       if (lseek(fd, xph_offset, SEEK_SET) == (off_t)-1) {
                                file_badseek(ms);
                                return -1;
                        }
@@ -1146,10 +1102,6 @@ dophn_exec(struct magic_set *ms, int clazz, int swap, int fd, off_t off,
                                if (offset == 0)
                                        break;
                        }
-                       if (lseek(fd, savedoffset, SEEK_SET) == (off_t)-1) {
-                               file_badseek(ms);
-                               return -1;
-                       }
                        break;
                default:
                        break;