aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShao Miller <sha0.miller@gmail.com>2012-11-02 11:59:10 -0400
committerShao Miller <sha0.miller@gmail.com>2012-11-29 13:40:12 -0500
commit37971728a5fc40b1c90512e79e47333d98ec8851 (patch)
tree2a933da7e9831fa02e097cf03d6edf87863a8724
parent7307d60063ee4303da4de45f9d984fdc8df92146 (diff)
downloadsyslinux-37971728a5fc40b1c90512e79e47333d98ec8851.tar.gz
syslinux-37971728a5fc40b1c90512e79e47333d98ec8851.tar.xz
syslinux-37971728a5fc40b1c90512e79e47333d98ec8851.zip
fs: Fix searchdir resource leak
This is a significant rewrite of the generic lookup logic inside core/fs/fs.c's searchdir function. Previously, there was a memory leak if a path involved multiple directories. After a sufficiently large number of invocations, this could be observed. Reported-by: Ady <ady-sf@hotmail.com> Signed-off-by: Shao Miller <sha0.miller@gmail.com>
-rw-r--r--core/fs/fs.c263
1 files changed, 155 insertions, 108 deletions
diff --git a/core/fs/fs.c b/core/fs/fs.c
index 21f5dba0..3cb27b0d 100644
--- a/core/fs/fs.c
+++ b/core/fs/fs.c
@@ -202,11 +202,10 @@ void pm_searchdir(com32sys_t *regs)
int searchdir(const char *name)
{
- struct inode *inode = NULL;
- struct inode *parent = NULL;
+ static char root_name[] = "/";
struct file *file;
- char *pathbuf = NULL;
- char *part, *p, echar;
+ char *path, *inode_name, *next_inode_name;
+ struct inode *tmp, *inode = NULL;
int symlink_count = MAX_SYMLINK_CNT;
dprintf("searchdir: %s root: %p cwd: %p\n",
@@ -228,113 +227,165 @@ int searchdir(const char *name)
/* else, try the generic-path-lookup method */
- parent = get_inode(this_fs->cwd);
- p = pathbuf = strdup(name);
- if (!pathbuf)
- goto err;
+ /* Copy the path */
+ path = strdup(name);
+ if (!path) {
+ dprintf("searchdir: Couldn't copy path\n");
+ goto err_path;
+ }
+
+ /* Work with the current directory, by default */
+ inode = get_inode(this_fs->cwd);
+ if (!inode) {
+ dprintf("searchdir: Couldn't use current directory\n");
+ goto err_curdir;
+ }
- do {
- got_link:
- if (*p == '/') {
- put_inode(parent);
- parent = get_inode(this_fs->root);
+ for (inode_name = path; inode_name; inode_name = next_inode_name) {
+ /* Root directory? */
+ if (inode_name[0] == '/') {
+ next_inode_name = inode_name + 1;
+ inode_name = root_name;
+ } else {
+ /* Find the next inode name */
+ next_inode_name = strchr(inode_name + 1, '/');
+ if (next_inode_name) {
+ /* Terminate the current inode name and point to next */
+ *next_inode_name++ = '\0';
+ }
+ }
+ if (next_inode_name) {
+ /* Advance beyond redundant slashes */
+ while (*next_inode_name == '/')
+ next_inode_name++;
+
+ /* Check if we're at the end */
+ if (*next_inode_name == '\0')
+ next_inode_name = NULL;
+ }
+ dprintf("searchdir: inode_name: %s\n", inode_name);
+ if (next_inode_name)
+ dprintf("searchdir: Remaining: %s\n", next_inode_name);
+
+ /* Root directory? */
+ if (inode_name[0] == '/') {
+ /* Release any chain that's already been established */
+ put_inode(inode);
+ inode = get_inode(this_fs->root);
+ continue;
}
- do {
- inode = get_inode(parent);
-
- while (*p == '/')
- p++;
-
- if (!*p)
- break;
-
- part = p;
- while ((echar = *p) && echar != '/')
- p++;
- *p++ = '\0';
-
- if (part[0] == '.' && part[1] == '.' && part[2] == '\0') {
- if (inode->parent) {
- put_inode(parent);
- parent = get_inode(inode->parent);
- put_inode(inode);
- inode = NULL;
- if (!echar) {
- /* Terminal double dots */
- inode = parent;
- parent = inode->parent ?
- get_inode(inode->parent) : NULL;
- }
- }
- } else if (part[0] != '.' || part[1] != '\0') {
- inode = this_fs->fs_ops->iget(part, parent);
- if (!inode)
- goto err;
- if (inode->mode == DT_LNK) {
- char *linkbuf, *q;
- int name_len = echar ? strlen(p) : 0;
- int total_len = inode->size + name_len + 2;
- int link_len;
-
- if (!this_fs->fs_ops->readlink ||
- --symlink_count == 0 || /* limit check */
- total_len > MAX_SYMLINK_BUF)
- goto err;
-
- linkbuf = malloc(total_len);
- if (!linkbuf)
- goto err;
-
- link_len = this_fs->fs_ops->readlink(inode, linkbuf);
- if (link_len <= 0) {
- free(linkbuf);
- goto err;
- }
-
- q = linkbuf + link_len;
-
- if (echar) {
- if (link_len > 0 && q[-1] != '/')
- *q++ = '/';
-
- memcpy(q, p, name_len+1);
- } else {
- *q = '\0';
- }
-
- free(pathbuf);
- p = pathbuf = linkbuf;
- put_inode(inode);
- inode = NULL;
- goto got_link;
- }
-
- inode->name = strdup(part);
- dprintf("path component: %s\n", inode->name);
-
- inode->parent = parent;
- parent = NULL;
-
- if (!echar)
- break;
-
- if (inode->mode != DT_DIR)
- goto err;
-
- parent = inode;
- inode = NULL;
+ /* Current directory? */
+ if (!strncmp(inode_name, ".", sizeof "."))
+ continue;
+
+ /* Parent directory? */
+ if (!strncmp(inode_name, "..", sizeof "..")) {
+ /* If there is no parent, just ignore it */
+ if (!inode->parent)
+ continue;
+
+ /* Add a reference to the parent so we can release the child */
+ tmp = get_inode(inode->parent);
+
+ /* Releasing the child will drop the parent back down to 1 */
+ put_inode(inode);
+
+ inode = tmp;
+ continue;
+ }
+
+ /* Anything else */
+ tmp = inode;
+ inode = this_fs->fs_ops->iget(inode_name, inode);
+ if (!inode) {
+ /* Failure. Release the chain */
+ put_inode(tmp);
+ break;
+ }
+
+ /* Sanity-check */
+ if (inode->parent && inode->parent != tmp) {
+ dprintf("searchdir: iget returned a different parent\n");
+ put_inode(inode);
+ inode = NULL;
+ put_inode(tmp);
+ break;
+ }
+ inode->parent = tmp;
+ inode->name = strdup(inode_name);
+ dprintf("searchdir: path component: %s\n", inode->name);
+
+ /* Symlink handling */
+ if (inode->mode == DT_LNK) {
+ char *new_path;
+ int new_len, copied;
+
+ /* target path + NUL */
+ new_len = inode->size + 1;
+
+ if (next_inode_name) {
+ /* target path + slash + remaining + NUL */
+ new_len += strlen(next_inode_name) + 1;
+ }
+
+ if (!this_fs->fs_ops->readlink ||
+ /* limit checks */
+ --symlink_count == 0 ||
+ new_len > MAX_SYMLINK_BUF)
+ goto err_new_len;
+
+ new_path = malloc(new_len);
+ if (!new_path)
+ goto err_new_path;
+
+ copied = this_fs->fs_ops->readlink(inode, new_path);
+ if (copied <= 0)
+ goto err_copied;
+ new_path[copied] = '\0';
+ dprintf("searchdir: Symlink: %s\n", new_path);
+
+ if (next_inode_name) {
+ new_path[copied] = '/';
+ strcpy(new_path + copied + 1, next_inode_name);
+ dprintf("searchdir: New path: %s\n", new_path);
}
- } while (echar);
- } while (0);
- free(pathbuf);
- pathbuf = NULL;
- put_inode(parent);
- parent = NULL;
+ free(path);
+ path = next_inode_name = new_path;
- if (!inode)
+ /* Add a reference to the parent so we can release the child */
+ tmp = get_inode(inode->parent);
+
+ /* Releasing the child will drop the parent back down to 1 */
+ put_inode(inode);
+
+ inode = tmp;
+ continue;
+err_copied:
+ free(new_path);
+err_new_path:
+err_new_len:
+ put_inode(inode);
+ inode = NULL;
+ break;
+ }
+
+ /* If there's more to process, this should be a directory */
+ if (next_inode_name && inode->mode != DT_DIR) {
+ dprintf("searchdir: Expected a directory\n");
+ put_inode(inode);
+ inode = NULL;
+ break;
+ }
+ }
+err_curdir:
+ free(path);
+err_path:
+ if (!inode) {
+ dprintf("searchdir: Not found\n");
goto err;
+ }
file->inode = inode;
file->offset = 0;
@@ -342,10 +393,6 @@ int searchdir(const char *name)
return file_to_handle(file);
err:
- put_inode(inode);
- put_inode(parent);
- if (pathbuf)
- free(pathbuf);
_close_file(file);
err_no_close:
return -1;