From 76a29519ff87dd6a014d841a3a6e501d3b2f5153 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Br=C3=BCns?= Date: Tue, 6 Sep 2016 04:36:41 +0200 Subject: ext4: fix possible crash on directory traversal, ignore deleted entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The following command triggers a segfault in search_dir: ./sandbox/u-boot -c 'host bind 0 ./sandbox/test/fs/3GB.ext4.img ; ext4write host 0 0 /./foo 0x10' The following command triggers a segfault in check_filename: ./sandbox/u-boot -c 'host bind 0 ./sandbox/test/fs/3GB.ext4.img ; ext4write host 0 0 /. 0x10' "." is the first entry in the directory, thus previous_dir is NULL. The whole previous_dir block in search_dir seems to be a bad copy from check_filename(...). As the changed data is not written to disk, the statement is mostly harmless, save the possible NULL-ptr reference. Typically a file is unlinked by extending the direntlen of the previous entry. If the entry is the first entry in the directory block, it is invalidated by setting inode=0. The inode==0 case is hard to trigger without crafted filesystems. It only hits if the first entry in a directory block is deleted and later a lookup for the entry (by name) is done. Signed-off-by: Stefan Brüns Reviewed-by: Lukasz Majewski --- fs/ext4/ext4_common.c | 58 +++++++++++++++++++-------------------------------- fs/ext4/ext4_write.c | 2 +- 2 files changed, 22 insertions(+), 38 deletions(-) (limited to 'fs/ext4') diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index cd1bdfe025..d647ae0fd2 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -533,16 +533,14 @@ fail: static int search_dir(struct ext2_inode *parent_inode, char *dirname) { int status; - int inodeno; + int inodeno = 0; int totalbytes; int templength; int direct_blk_idx; long int blknr; - int found = 0; char *ptr = NULL; unsigned char *block_buffer = NULL; struct ext2_dirent *dir = NULL; - struct ext2_dirent *previous_dir = NULL; struct ext_filesystem *fs = get_fs(); /* read the block no allocated to a file */ @@ -552,7 +550,7 @@ static int search_dir(struct ext2_inode *parent_inode, char *dirname) if (blknr == 0) goto fail; - /* read the blocks of parenet inode */ + /* read the blocks of parent inode */ block_buffer = zalloc(fs->blksz); if (!block_buffer) goto fail; @@ -572,15 +570,9 @@ static int search_dir(struct ext2_inode *parent_inode, char *dirname) * space in the block that means * it is a last entry of directory entry */ - if (strlen(dirname) == dir->namelen) { + if (dir->inode && (strlen(dirname) == dir->namelen)) { if (strncmp(dirname, ptr + sizeof(struct ext2_dirent), dir->namelen) == 0) { - uint16_t new_len; - new_len = le16_to_cpu(previous_dir->direntlen); - new_len += le16_to_cpu(dir->direntlen); - previous_dir->direntlen = cpu_to_le16(new_len); inodeno = le32_to_cpu(dir->inode); - dir->inode = 0; - found = 1; break; } } @@ -591,19 +583,15 @@ static int search_dir(struct ext2_inode *parent_inode, char *dirname) /* traversing the each directory entry */ templength = le16_to_cpu(dir->direntlen); totalbytes = totalbytes + templength; - previous_dir = dir; dir = (struct ext2_dirent *)((char *)dir + templength); ptr = (char *)dir; } - if (found == 1) { - free(block_buffer); - block_buffer = NULL; - return inodeno; - } - free(block_buffer); block_buffer = NULL; + + if (inodeno > 0) + return inodeno; } fail: @@ -779,15 +767,13 @@ fail: return result_inode_no; } -static int check_filename(char *filename, unsigned int blknr) +static int unlink_filename(char *filename, unsigned int blknr) { - unsigned int first_block_no_of_root; int totalbytes = 0; int templength = 0; int status, inodeno; int found = 0; char *root_first_block_buffer = NULL; - char *root_first_block_addr = NULL; struct ext2_dirent *dir = NULL; struct ext2_dirent *previous_dir = NULL; char *ptr = NULL; @@ -795,18 +781,15 @@ static int check_filename(char *filename, unsigned int blknr) int ret = -1; /* get the first block of root */ - first_block_no_of_root = blknr; root_first_block_buffer = zalloc(fs->blksz); if (!root_first_block_buffer) return -ENOMEM; - root_first_block_addr = root_first_block_buffer; - status = ext4fs_devread((lbaint_t)first_block_no_of_root * - fs->sect_perblk, 0, + status = ext4fs_devread((lbaint_t)blknr * fs->sect_perblk, 0, fs->blksz, root_first_block_buffer); if (status == 0) goto fail; - if (ext4fs_log_journal(root_first_block_buffer, first_block_no_of_root)) + if (ext4fs_log_journal(root_first_block_buffer, blknr)) goto fail; dir = (struct ext2_dirent *)root_first_block_buffer; ptr = (char *)dir; @@ -818,19 +801,21 @@ static int check_filename(char *filename, unsigned int blknr) * is free availble space in the block that * means it is a last entry of directory entry */ - if (strlen(filename) == dir->namelen) { - if (strncmp(filename, ptr + sizeof(struct ext2_dirent), - dir->namelen) == 0) { + if (dir->inode && (strlen(filename) == dir->namelen) && + (strncmp(ptr + sizeof(struct ext2_dirent), + filename, dir->namelen) == 0)) { + printf("file found, deleting\n"); + inodeno = le32_to_cpu(dir->inode); + if (previous_dir) { uint16_t new_len; - printf("file found deleting\n"); new_len = le16_to_cpu(previous_dir->direntlen); new_len += le16_to_cpu(dir->direntlen); previous_dir->direntlen = cpu_to_le16(new_len); - inodeno = le32_to_cpu(dir->inode); + } else { dir->inode = 0; - found = 1; - break; } + found = 1; + break; } if (fs->blksz - totalbytes == le16_to_cpu(dir->direntlen)) @@ -846,8 +831,7 @@ static int check_filename(char *filename, unsigned int blknr) if (found == 1) { - if (ext4fs_put_metadata(root_first_block_addr, - first_block_no_of_root)) + if (ext4fs_put_metadata(root_first_block_buffer, blknr)) goto fail; ret = inodeno; } @@ -857,7 +841,7 @@ fail: return ret; } -int ext4fs_filename_check(char *filename) +int ext4fs_filename_unlink(char *filename) { short direct_blk_idx = 0; long int blknr = -1; @@ -869,7 +853,7 @@ int ext4fs_filename_check(char *filename) blknr = read_allocated_block(g_parent_inode, direct_blk_idx); if (blknr == 0) break; - inodeno = check_filename(filename, blknr); + inodeno = unlink_filename(filename, blknr); if (inodeno != -1) return inodeno; } diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c index fac3222ef8..9200c4727e 100644 --- a/fs/ext4/ext4_write.c +++ b/fs/ext4/ext4_write.c @@ -882,7 +882,7 @@ int ext4fs_write(const char *fname, unsigned char *buffer, if (ext4fs_iget(parent_inodeno, g_parent_inode)) goto fail; /* check if the filename is already present in root */ - existing_file_inodeno = ext4fs_filename_check(filename); + existing_file_inodeno = ext4fs_filename_unlink(filename); if (existing_file_inodeno != -1) { ret = ext4fs_delete_file(existing_file_inodeno); fs->first_pass_bbmap = 0; -- cgit v1.2.3