core: fix double free of pathbuf, constify iget filename, fix iso9660

Fix double free of pathbuf in searchdir().
Constify the pathname passed to ->iget().
Major cleanups of the iso9660 filesystem; it would use insufficiently
dimensioned stack variables and do compares in a rather inefficient
manner.

Signed-off-by: H. Peter Anvin <hpa@zytor.com>

7 files changed, 87 insertions, 135 deletions

diff --git a/core/Makefile b/core/Makefile
index 7243ccd5..a5d0fdd5 100644
--- a/core/Makefile
+++ b/core/Makefile
@@ -58,6 +58,8 @@ NASMOPT  += $(NASMDEBUG)
 
 PREPCORE = ../lzo/prepcore
 
+# CFLAGS	+= -DDEBUG=1
+
 # The DATE is set on the make command line when building binaries for
 # official release.  Otherwise, substitute a hex string that is pretty much
 # guaranteed to be unique to be unique from build to build.
@@ -131,7 +133,7 @@ tidy dist:
 	rm -f *.elf.tmp *.sym
 	rm -f *.lsr *.lst *.map *.sec *.raw
 	rm -f */*.o */*/*.o */*.lst */*/*.lst */.*.d */*/.*.d
-	rm -f $(OBSOLETE)
+	rm -f $(OBSOLETE) $(LIB)
 
 clean: tidy
 
diff --git a/core/fs/btrfs/btrfs.c b/core/fs/btrfs/btrfs.c
index 5a53c2c7..a602cd6f 100644
--- a/core/fs/btrfs/btrfs.c
+++ b/core/fs/btrfs/btrfs.c
@@ -487,7 +487,7 @@ static struct inode *btrfs_iget_root(struct fs_info *fs)
 	return btrfs_iget_by_inr(fs, BTRFS_FIRST_CHUNK_TREE_OBJECTID);
 }
 
-static struct inode *btrfs_iget(char *name, struct inode *parent)
+static struct inode *btrfs_iget(const char *name, struct inode *parent)
 {
 	struct fs_info *fs = parent->fs;
 	struct btrfs_disk_key search_key;
diff --git a/core/fs/ext2/ext2.c b/core/fs/ext2/ext2.c
index 060c3e48..5e806ee3 100644
--- a/core/fs/ext2/ext2.c
+++ b/core/fs/ext2/ext2.c
@@ -275,7 +275,7 @@ static struct inode *ext2_iget_root(struct fs_info *fs)
     return ext2_iget_by_inr(fs, EXT2_ROOT_INO);
 }
 
-static struct inode *ext2_iget(char *dname, struct inode *parent)
+static struct inode *ext2_iget(const char *dname, struct inode *parent)
 {
     const struct ext2_dir_entry *de;
     struct fs_info *fs = parent->fs;
@@ -322,7 +322,6 @@ int ext2_readlink(struct inode *inode, char *buf)
     struct fs_info *fs = inode->fs;
     int sec_per_block = 1 << (fs->block_shift - fs->sector_shift);
     bool fast_symlink;
-    const char *data;
 
     if (inode->size > BLOCK_SIZE(fs))
 	return -1;		/* Error! */
@@ -340,7 +339,7 @@ int ext2_readlink(struct inode *inode, char *buf)
 /*
  * Read one directory entry at a time
  */
-static struct dirent * ext2_readdir(struct file *file)
+static struct dirent *ext2_readdir(struct file *file)
 {
     struct fs_info *fs = file->fs;
     struct inode *inode = file->inode;
diff --git a/core/fs/fat/fat.c b/core/fs/fat/fat.c
index ba4ae1a1..03c6f05c 100644
--- a/core/fs/fat/fat.c
+++ b/core/fs/fat/fat.c
@@ -296,7 +296,7 @@ static void vfat_mangle_name(char *dst, const char *src)
 /*
  * Mangle a normal style string to DOS style string.
  */
-static void mangle_dos_name(char *mangle_buf, char *src)
+static void mangle_dos_name(char *mangle_buf, const char *src)
 {
     int i;
     unsigned char c;
@@ -443,7 +443,7 @@ static inline int get_inode_mode(uint8_t attr)
 }
 
 
-static struct inode *vfat_find_entry(char *dname, struct inode *dir)
+static struct inode *vfat_find_entry(const char *dname, struct inode *dir)
 {
     struct fs_info *fs = dir->fs;
     struct inode *inode;
@@ -577,7 +577,7 @@ static struct inode *vfat_iget_root(struct fs_info *fs)
     return inode;
 }
 
-static struct inode *vfat_iget(char *dname, struct inode *parent)
+static struct inode *vfat_iget(const char *dname, struct inode *parent)
 {
     return vfat_find_entry(dname, parent);
 }
diff --git a/core/fs/fs.c b/core/fs/fs.c
index e0f469ba..c24feede 100644
--- a/core/fs/fs.c
+++ b/core/fs/fs.c
@@ -232,7 +232,7 @@ int searchdir(const char *name)
 		if (inode->mode == I_SYMLINK) {
 		    char *linkbuf, *q;
 		    int name_len = echar ? strlen(p) : 0;
-		    int total_len = inode->size + name_len + (echar ? 2 : 1);
+		    int total_len = inode->size + name_len + 2;
 		    int link_len;
 
 		    if (!this_fs->fs_ops->readlink ||
@@ -284,6 +284,7 @@ int searchdir(const char *name)
     } while (0);
 
     free(pathbuf);
+    pathbuf = NULL;
     put_inode(parent);
     parent = NULL;
 
diff --git a/core/fs/iso9660/iso9660.c b/core/fs/iso9660/iso9660.c
index 61860419..fa7fd2d7 100644
--- a/core/fs/iso9660/iso9660.c
+++ b/core/fs/iso9660/iso9660.c
@@ -1,3 +1,4 @@
+#include <dprintf.h>
 #include <stdio.h>
 #include <string.h>
 #include <sys/dirent.h>
@@ -7,6 +8,15 @@
 #include <fs.h>
 #include "iso9660_fs.h"
 
+/* Convert to lower case string */
+static inline char iso_tolower(char c)
+{
+    if (c >= 'A' && c <= 'Z')
+	c += 0x20;
+
+    return c;
+}
+
 static struct inode *new_iso_inode(struct fs_info *fs)
 {
     return alloc_inode(fs, 0, sizeof(uint32_t));
@@ -63,65 +73,66 @@ static void iso_mangle_name(char *dst, const char *src)
         *dst++ = '\0';
 }
 
-static int iso_convert_name(char *dst, const char *src, int len)
+static size_t iso_convert_name(char *dst, const char *src, int len)
 {
-    int i = 0;
+    char *p = dst;
     char c;
     
-    for (; i < len; i++) {
-	c = src[i];
-	if (!c)
+    if (len == 1) {
+	switch (*src) {
+	case 1:
+	    *p++ = '.';
+	    /* fall through */
+	case 0:
+	    *p++ = '.';
+	    goto done;
+	default:
+	    /* nothing special */
 	    break;
-	
-	/* remove ';1' in the end */
-	if (c == ';' && i == len - 2 && src[i + 1] == '1')
-	    break;
-	/* convert others ';' to '.' */
-	if (c == ';')
-	    c = '.';
-	*dst++ = c;
+	}
     }
-    
-    /* Then remove the terminal dots */
-    while (*(dst - 1) == '.') {
-	if (i <= 2)
+
+    while ((c = *src++)) {
+	/* Remove filename version suffix */
+	if (c == ';')
 	    break;
-	dst--;
-	i--;
+	*p++ = iso_tolower(c);
     }
-    *dst = 0;
     
-    return i;
+    /* Then remove any terminal dots */
+    while (p > dst+1 && p[-1] == '.')
+	p--;
+
+done:
+    *p = '\0';
+    return p - dst;
 }
 
 /* 
  * Unlike strcmp, it does return 1 on match, or reutrn 0 if not match.
  */
-static int iso_compare_name(const char *de_name, int len,
-			    const char *file_name)
+static bool iso_compare_name(const char *de_name, size_t len,
+			     const char *file_name)
 {
     char iso_file_name[256];
     char *p = iso_file_name;
     char c1, c2;
-    int i;
+    size_t i;
     
     i = iso_convert_name(iso_file_name, de_name, len);
-    
-    if (i != (int)strlen(file_name))
-	return 0;
-    
-    while (i--) {
+    dprintf("Compare: \"%s\" to \"%s\" (len %zu)\n",
+	    file_name, iso_file_name, i);
+
+    do {
 	c1 = *p++;
-	c2 = *file_name++;
-	
-	/* convert to lower case */
-	c1 |= 0x20;
-	c2 |= 0x20;
+	c2 = iso_tolower(*file_name++);
+
+	/* compare equal except for case? */
 	if (c1 != c2)
-	    return 0;
-    }
-    
-    return 1;
+	    return false;
+    } while (c1);
+
+    return true;
 }
 
 static inline int cdrom_read_blocks(struct disk *disk, void *buf, 
@@ -164,7 +175,7 @@ static uint32_t iso_getfssec(struct file *file, char *buf,
  * Find a entry in the specified dir with name _dname_.
  */
 static const struct iso_dir_entry *
-iso_find_entry(char *dname, struct inode *inode)
+iso_find_entry(const char *dname, struct inode *inode)
 {
     struct fs_info *fs = inode->fs;
     block_t dir_block = *(uint32_t *)inode->pvt;
@@ -172,59 +183,41 @@ iso_find_entry(char *dname, struct inode *inode)
     const char *de_name;
     int de_name_len, de_len;
     const struct iso_dir_entry *de;
-    struct iso_dir_entry tmpde;
     const char *data = NULL;
-    struct cache_struct *cs = NULL;
+
+    dprintf("iso_find_entry: \"%s\"\n", dname);
     
     while (1) {
 	if (!data) {
+	    dprintf("Getting block %d from block %llu\n", i, dir_block);
 	    if (++i > inode->blocks)
-		return NULL;
+		return NULL;	/* End of directory */
 	    data = get_cache(fs->fs_dev, dir_block++);
-	    de = (const struct iso_dir_entry *)data;
 	    offset = 0;
 	}
+
 	de = (const struct iso_dir_entry *)(data + offset);
-	
 	de_len = de->length;
-	if (de_len == 0) {    /* move on to the next block */
-	    cs = NULL;
-	    continue;
-	}
 	offset += de_len;
 	
 	/* Make sure we have a full directory entry */
-	if (offset >= BLOCK_SIZE(fs)) {
-	    int slop = de_len + BLOCK_SIZE(fs) - offset;
-	    
-	    memcpy(&tmpde, de, slop);
-	    offset &= BLOCK_SIZE(fs) - 1;
-	    if (offset) {
-		if (++i > inode->blocks)
-		    return NULL;
-		data = get_cache(fs->fs_dev, dir_block++);
-		memcpy((void *)&tmpde + slop, data, offset);
-	    }
-	    de = &tmpde;
-	}
-	
-	if (de_len < 33) {
-	    printf("Corrupted directory entry in sector %u\n", 
-		   (uint32_t)(dir_block - 1));
-	    return NULL;
+	if (de_len < 33 || offset > BLOCK_SIZE(fs)) {
+	    /*
+	     * Zero = end of sector, or corrupt directory entry
+	     *
+	     * ECMA-119:1987 6.8.1.1: "Each Directory Record shall end
+	     * in the Logical Sector in which it begins.
+	     */
+	    data = NULL;
+	    continue;
 	}
 	
 	de_name_len = de->name_len;
 	de_name = de->name;
-	/* Handling the special case ".' and '..' here */
-	if((de_name_len == 1) && (*de_name == 0)) {
-	    de_name = ".";
-	} else if ((de_name_len == 1) && (*de_name == 1)) {
-	    de_name ="..";
-	    de_name_len = 2;
-	}
-	if (iso_compare_name(de_name, de_name_len, dname))
+	if (iso_compare_name(de_name, de_name_len, dname)) {
+	    dprintf("Found.\n");
 	    return de;
+	}
     }
 }
 
@@ -270,7 +263,7 @@ static struct inode *iso_iget_root(struct fs_info *fs)
     return inode;
 }	
 
-static struct inode *iso_iget(char *dname, struct inode *parent)
+static struct inode *iso_iget(const char *dname, struct inode *parent)
 {
     const struct iso_dir_entry *de;
     
@@ -281,30 +274,18 @@ static struct inode *iso_iget(char *dname, struct inode *parent)
     return iso_get_inode(parent->fs, de);
 }
 
-/* Convert to lower case string */
-static void tolower_str(char *str)
-{
-	while (*str) {
-		if (*str >= 'A' && *str <= 'Z')
-			*str = *str + 0x20;
-		str++;
-	}
-}
-
 static struct dirent *iso_readdir(struct file *file)
 {
     struct fs_info *fs = file->fs;
     struct inode *inode = file->inode;
     const struct iso_dir_entry *de;
-    struct iso_dir_entry tmpde;
     struct dirent *dirent;
     const char *data = NULL;
-    block_t block =  *(uint32_t *)file->inode->pvt
-	+ (file->offset >> fs->block_shift);
     int offset = file->offset & (BLOCK_SIZE(fs) - 1);
-    int i = 0;
+    int i = file->offset >> BLOCK_SHIFT(fs);
+    block_t block =  *(uint32_t *)file->inode->pvt + i;
     int de_len, de_name_len;
-    char *de_name;
+    const char *de_name;
     
     while (1) {
 	if (!data) {
@@ -315,45 +296,16 @@ static struct dirent *iso_readdir(struct file *file)
 	de = (const struct iso_dir_entry *)(data + offset);
 	
 	de_len = de->length;
-	if (de_len == 0) {    /* move on to the next block */
+	offset += de_len;
+	if (de_len < 33 || offset > BLOCK_SIZE(fs)) {
 	    data = NULL;
 	    file->offset = (file->offset + BLOCK_SIZE(fs) - 1)
-		>> fs->block_shift;
+		& ~(BLOCK_SIZE(fs) - 1);
 	    continue;
 	}
-	offset += de_len;
-	
-	/* Make sure we have a full directory entry */
-	if (offset >= BLOCK_SIZE(fs)) {
-	    int slop = de_len + BLOCK_SIZE(fs) - offset;
-	    
-	    memcpy(&tmpde, de, slop);
-	    offset &= BLOCK_SIZE(fs) - 1;
-	    if (offset) {
-		if (++i > inode->blocks)
-		    return NULL;
-		data = get_cache(fs->fs_dev, block++);
-		memcpy((char *)&tmpde + slop, data, offset);
-	    }
-	    de = &tmpde;
-	}
-	
-	if (de_len < 33) {
-	    printf("Corrupted directory entry in sector %u\n", 
-		   (uint32_t)(block - 1));
-	    return NULL;
-	}
 	
 	de_name_len = de->name_len;
 	de_name = de->name;
-	/* Handling the special case ".' and '..' here */
-	if((de_name_len == 1) && (*de_name == 0)) {
-	    de_name = ".";
-	} else if ((de_name_len == 1) && (*de_name == 1)) {
-	    de_name ="..";
-	    de_name_len = 2;
-	}
-	
 	break;
     }
     
@@ -364,10 +316,8 @@ static struct dirent *iso_readdir(struct file *file)
     
     dirent->d_ino = 0;           /* Inode number is invalid to ISO fs */
     dirent->d_off = file->offset;
-    dirent->d_reclen = de_len;
     dirent->d_type = get_inode_mode(de->flags);
-    iso_convert_name(dirent->d_name, de_name, de_name_len);
-    tolower_str(dirent->d_name);
+    dirent->d_reclen = iso_convert_name(dirent->d_name, de_name, de_name_len);
     
     file->offset += de_len;  /* Update for next reading */
     
diff --git a/core/include/fs.h b/core/include/fs.h
index 1f4ad280..25219d96 100644
--- a/core/include/fs.h
+++ b/core/include/fs.h
@@ -64,7 +64,7 @@ struct fs_ops {
     int      (*load_config)(void);
 
     struct inode * (*iget_root)(struct fs_info *);
-    struct inode * (*iget)(char *, struct inode *);
+    struct inode * (*iget)(const char *, struct inode *);
     int	     (*readlink)(struct inode *, char *);
 
     /* the _dir_ stuff */