[syslinux:pathbased] core: fix double free of pathbuf, constify iget filename, fix iso9660
syslinux-bot for H. Peter Anvin
hpa at zytor.com
Wed Feb 24 18:30:09 PST 2010
Commit-ID: 6b3041d5e663ebeb162782d44b1d92fe2bd35fdf
Gitweb: http://syslinux.zytor.com/commit/6b3041d5e663ebeb162782d44b1d92fe2bd35fdf
Author: H. Peter Anvin <hpa at zytor.com>
AuthorDate: Wed, 24 Feb 2010 18:24:14 -0800
Committer: H. Peter Anvin <hpa at zytor.com>
CommitDate: Wed, 24 Feb 2010 18:24:14 -0800
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 at zytor.com>
---
core/Makefile | 4 +-
core/fs/btrfs/btrfs.c | 2 +-
core/fs/ext2/ext2.c | 5 +-
core/fs/fat/fat.c | 6 +-
core/fs/fs.c | 3 +-
core/fs/iso9660/iso9660.c | 200 +++++++++++++++++----------------------------
core/include/fs.h | 2 +-
7 files changed, 87 insertions(+), 135 deletions(-)
diff --git a/core/Makefile b/core/Makefile
index 7243ccd..a5d0fdd 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 5a53c2c..a602cd6 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 060c3e4..5e806ee 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 ba4ae1a..03c6f05 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 e0f469b..c24feed 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 6186041..fa7fd2d 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 1f4ad28..25219d9 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 */
More information about the Syslinux-commits
mailing list