Boot loader fixes - fix recursive malloc()/free() errors, NULL freed fields
authorMatthew Dillon <dillon@apollo.backplane.com>
Sun, 15 Feb 2009 07:45:22 +0000 (23:45 -0800)
committerMatthew Dillon <dillon@apollo.backplane.com>
Sun, 15 Feb 2009 07:45:22 +0000 (23:45 -0800)
* Fix reported loader panics related to corrupt malloc areas.  The zip/gzip
  modules were using a static variable to hold malloc()ed space.  The
  field was getting tromped by recursion.

* Fix numerous cases where file structure fields are not NULL'd out upon
  release.

* Fix numerous cases where a double close might result in a double free.

* Fix a benign bug in libstand's realloc().

13 files changed:
lib/libstand/bzipfs.c
lib/libstand/cd9660.c
lib/libstand/close.c
lib/libstand/dosfs.c
lib/libstand/ext2fs.c
lib/libstand/gzipfs.c
lib/libstand/hammerread.c
lib/libstand/open.c
lib/libstand/tftp.c
lib/libstand/ufs.c
lib/libstand/zalloc_malloc.c
lib/libstand/zipfs.c
sys/boot/common/devopen.c

index 0757975..7de3a8c 100644 (file)
@@ -206,9 +206,12 @@ bzf_close(struct open_file *f)
 {
     struct bz_file     *bzf = (struct bz_file *)f->f_fsdata;
     
-    BZ2_bzDecompressEnd(&(bzf->bzf_bzstream));
-    close(bzf->bzf_rawfd);
-    free(bzf);
+    f->f_fsdata = NULL;
+    if (bzf) {
+       BZ2_bzDecompressEnd(&(bzf->bzf_bzstream));
+       close(bzf->bzf_rawfd);
+       free(bzf);
+    }
     return(0);
 }
  
index 20ab31b..53feaa6 100644 (file)
@@ -416,6 +416,7 @@ cd9660_open(const char *path, struct open_file *f)
        return 0;
 
 out:
+       f->f_fsdata = NULL;
        if (fp)
                free(fp);
        free(buf);
@@ -428,8 +429,9 @@ cd9660_close(struct open_file *f)
 {
        struct file *fp = (struct file *)f->f_fsdata;
 
-       f->f_fsdata = 0;
-       free(fp);
+       f->f_fsdata = NULL;
+       if (fp)
+               free(fp);
 
        return 0;
 }
index 0c45236..11bdca8 100644 (file)
@@ -78,8 +78,10 @@ close(int fd)
        errno = EBADF;
        return (-1);
     }
-    if (f->f_rabuf != NULL)
+    if (f->f_rabuf != NULL) {
        free(f->f_rabuf);
+       f->f_rabuf = NULL;
+    }
     if (!(f->f_flags & F_RAW) && f->f_ops)
        err1 = (f->f_ops->fo_close)(f);
     if (!(f->f_flags & F_NODEV) && f->f_dev)
index e55dc7d..0c966c4 100644 (file)
@@ -325,11 +325,16 @@ static int
 dos_close(struct open_file *fd)
 {
     DOS_FILE *f = (DOS_FILE *)fd->f_fsdata;
-    DOS_FS *fs = f->fs;
+    DOS_FS *fs;
 
-    f->fs->links--;
-    free(f);
-    dos_unmount(fs);
+    fd->f_fsdata = NULL;
+    if (f) {
+       fs = f->fs;
+       f->fs = NULL;
+       fs->links--;
+       free(f);
+       dos_unmount(fs);
+    }
     return 0;
 }
 
index cc612a1..fc0f16a 100644 (file)
@@ -541,6 +541,7 @@ out:
        if (path)
                free(path);
        if (error) {
+               f->f_fsdata = NULL;
                if (fp->f_buf)
                        free(fp->f_buf);
                free(fp->f_fs);
index f5b1c89..7ea8708 100644 (file)
@@ -162,7 +162,7 @@ check_header(struct z_file *zf)
 static int
 zf_open(const char *fname, struct open_file *f)
 {
-    static char                *zfname;
+    char               *zfname;
     int                        rawfd;
     struct z_file      *zf;
     char               *cp;
@@ -183,7 +183,6 @@ zf_open(const char *fname, struct open_file *f)
     if (zfname == NULL)
         return(ENOMEM);
     sprintf(zfname, "%s.gz", fname);
-
     /* Try to open the compressed datafile */
     rawfd = open(zfname, O_RDONLY);
     free(zfname);
index 86e86ae..b24a57b 100644 (file)
@@ -818,20 +818,22 @@ hinit(struct hfs *hfs)
        hfs->lru = 0;
 
        hammer_volume_ondisk_t volhead = hread(hfs, HAMMER_ZONE_ENCODE(1, 0));
-       if (volhead == NULL)
-               return (-1);
 
 #ifdef TESTING
-       printf("signature: %svalid\n",
-              volhead->vol_signature != HAMMER_FSBUF_VOLUME ?
-                       "in" :
-                       "");
-       printf("name: %s\n", volhead->vol_name);
+       if (volhead) {
+               printf("signature: %svalid\n",
+                      volhead->vol_signature != HAMMER_FSBUF_VOLUME ?
+                               "in" :
+                               "");
+               printf("name: %s\n", volhead->vol_name);
+       }
 #endif
 
-       if (volhead->vol_signature != HAMMER_FSBUF_VOLUME) {
-               for (int i = 0; i < NUMCACHE; i++)
+       if (volhead == NULL || volhead->vol_signature != HAMMER_FSBUF_VOLUME) {
+               for (int i = 0; i < NUMCACHE; i++) {
                        free(hfs->cache[i].data);
+                       hfs->cache[i].data = NULL;
+               }
                errno = ENODEV;
                return (-1);
        }
@@ -848,8 +850,12 @@ hclose(struct hfs *hfs)
 #if DEBUG
        printf("hclose\n");
 #endif
-       for (int i = 0; i < NUMCACHE; i++)
-               free(hfs->cache[i].data);
+       for (int i = 0; i < NUMCACHE; i++) {
+               if (hfs->cache[i].data) {
+                       free(hfs->cache[i].data);
+                       hfs->cache[i].data = NULL;
+               }
+       }
 }
 #endif
 
@@ -864,14 +870,15 @@ static int
 hammer_open(const char *path, struct open_file *f)
 {
        struct hfile *hf = malloc(sizeof(*hf));
-       bzero(hf, sizeof(*hf));
 
+       bzero(hf, sizeof(*hf));
        f->f_fsdata = hf;
        hf->hfs.f = f;
        f->f_offset = 0;
 
        int rv = hinit(&hf->hfs);
        if (rv) {
+               f->f_fsdata = NULL;
                free(hf);
                return (rv);
        }
@@ -899,6 +906,7 @@ fail:
 #if DEBUG
        printf("hammer_open fail\n");
 #endif
+       f->f_fsdata = NULL;
        hclose(&hf->hfs);
        free(hf);
        return (ENOENT);
index 74f4372..90b603e 100644 (file)
@@ -107,14 +107,15 @@ open(const char *fname, int mode)
     f->f_ops = (struct fs_ops *)0;
     f->f_offset = 0;
     f->f_devdata = NULL;
-    file = (char *)0;
+    f->f_fsdata = NULL;
+    file = NULL;
     error = devopen(f, fname, &file);
     if (error ||
        (((f->f_flags & F_NODEV) == 0) && f->f_dev == (struct devsw *)0))
        goto err;
 
     /* see if we opened a raw device; otherwise, 'file' is the file name. */
-    if (file == (char *)0 || *file == '\0') {
+    if (file == NULL || *file == '\0') {
        f->f_flags |= F_RAW;
        return (fd);
     }
@@ -122,10 +123,8 @@ open(const char *fname, int mode)
     /* pass file name to the different filesystem open routines */
     besterror = ENOENT;
     for (i = 0; file_system[i] != NULL; i++) {
-
        error = ((*file_system[i]).fo_open)(file, f);
        if (error == 0) {
-           
            f->f_ops = file_system[i];
            o_rainit(f);
            return (fd);
index 0e09eac..98f66a2 100644 (file)
@@ -354,7 +354,7 @@ tftp_close(struct open_file *f)
        tftpfile = (struct tftp_handle *) f->f_fsdata;
 
        /* let it time out ... */
-
+       f->f_fsdata = NULL;
        if (tftpfile) {
                free(tftpfile->path);
                free(tftpfile);
index f59cd29..3f2afa1 100644 (file)
@@ -581,6 +581,7 @@ out:
        if (path)
                free(path);
        if (rc) {
+               f->f_fsdata = NULL;
                if (fp->f_buf)
                        free(fp->f_buf);
                free(fp->f_fs);
index 9edc047..174f9c3 100644 (file)
@@ -94,7 +94,7 @@ free(void *ptr)
 
 #ifdef USEGUARD
        if (res->ga_Magic != GAMAGIC)
-           panic("free: guard1 fail @ %p", ptr);
+           panic("free: guard1x fail @ %p", ptr);
        res->ga_Magic = -1;
 #endif
 #ifdef USEENDGUARD
@@ -143,6 +143,9 @@ realloc(void *ptr, size_t size)
     if ((res = malloc(size)) != NULL) {
        if (ptr) {
            old = *(size_t *)((char *)ptr - MALLOCALIGN) - MALLOCALIGN;
+#ifdef USEENDGUARD
+           --old;
+#endif
            if (old < size)
                bcopy(ptr, res, old);
            else
index 5a725dc..798b148 100644 (file)
@@ -163,7 +163,7 @@ check_header(struct z_file *zf)
 static int
 zf_open(const char *fname, struct open_file *f)
 {
-    static char                *zfname;
+    char               *zfname;
     int                        rawfd;
     struct z_file      *zf;
     char               *cp;
index d9e4ce3..8b26fbf 100644 (file)
@@ -56,6 +56,7 @@ devclose(struct open_file *f)
 {
     if (f->f_devdata != NULL) {
        free(f->f_devdata);
+       f->f_devdata = NULL;
     }
     return(0);
 }