* [U-Boot] [PATCH 1/3 v2] ext4: fix possible crash on directory traversal, ignore deleted entries [not found] <20160814144143.24301-1-stefan.bruens@rwth-aachen.de> @ 2016-08-14 14:41 ` Stefan Brüns 2016-08-18 14:44 ` Lukasz Majewski 2016-08-14 14:41 ` [U-Boot] [PATCH 2/3] ext4: propagate error if creation of directory entry fails Stefan Brüns 2016-08-14 14:41 ` [U-Boot] [PATCH 3/3] ext4: Do not crash when trying to grow a directory using extents Stefan Brüns 2 siblings, 1 reply; 6+ messages in thread From: Stefan Brüns @ 2016-08-14 14:41 UTC (permalink / raw) To: u-boot 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 <stefan.bruens@rwth-aachen.de> --- v2: Fix bad filename compare on delete, used substring only fs/ext4/ext4_common.c | 58 +++++++++++++++++++-------------------------------- fs/ext4/ext4_write.c | 2 +- include/ext4fs.h | 2 +- 3 files changed, 23 insertions(+), 39 deletions(-) diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index b00b84f..3ecd9a8 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -511,16 +511,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 */ @@ -530,7 +528,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; @@ -550,15 +548,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; } } @@ -569,19 +561,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: @@ -757,15 +745,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; @@ -773,18 +759,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; @@ -796,19 +779,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)) @@ -824,8 +809,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; } @@ -835,7 +819,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; @@ -847,7 +831,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 d03d692..f5811aa 100644 --- a/fs/ext4/ext4_write.c +++ b/fs/ext4/ext4_write.c @@ -865,7 +865,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; diff --git a/include/ext4fs.h b/include/ext4fs.h index 13d2c56..e3f6216 100644 --- a/include/ext4fs.h +++ b/include/ext4fs.h @@ -124,7 +124,7 @@ extern int gindex; int ext4fs_init(void); void ext4fs_deinit(void); -int ext4fs_filename_check(char *filename); +int ext4fs_filename_unlink(char *filename); int ext4fs_write(const char *fname, unsigned char *buffer, unsigned long sizebytes); int ext4_write_file(const char *filename, void *buf, loff_t offset, loff_t len, -- 2.9.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/3 v2] ext4: fix possible crash on directory traversal, ignore deleted entries 2016-08-14 14:41 ` [U-Boot] [PATCH 1/3 v2] ext4: fix possible crash on directory traversal, ignore deleted entries Stefan Brüns @ 2016-08-18 14:44 ` Lukasz Majewski 0 siblings, 0 replies; 6+ messages in thread From: Lukasz Majewski @ 2016-08-18 14:44 UTC (permalink / raw) To: u-boot Hi Stefan, > 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 <stefan.bruens@rwth-aachen.de> > --- > v2: Fix bad filename compare on delete, used substring only > > fs/ext4/ext4_common.c | 58 > +++++++++++++++++++-------------------------------- > fs/ext4/ext4_write.c | 2 +- include/ext4fs.h | 2 +- > 3 files changed, 23 insertions(+), 39 deletions(-) > > diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c > index b00b84f..3ecd9a8 100644 > --- a/fs/ext4/ext4_common.c > +++ b/fs/ext4/ext4_common.c > @@ -511,16 +511,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 */ > @@ -530,7 +528,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; > @@ -550,15 +548,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; > } > } > @@ -569,19 +561,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: > @@ -757,15 +745,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; > @@ -773,18 +759,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; > @@ -796,19 +779,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)) @@ -824,8 +809,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; > } > @@ -835,7 +819,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; > @@ -847,7 +831,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 d03d692..f5811aa 100644 > --- a/fs/ext4/ext4_write.c > +++ b/fs/ext4/ext4_write.c > @@ -865,7 +865,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; > diff --git a/include/ext4fs.h b/include/ext4fs.h > index 13d2c56..e3f6216 100644 > --- a/include/ext4fs.h > +++ b/include/ext4fs.h > @@ -124,7 +124,7 @@ extern int gindex; > > int ext4fs_init(void); > void ext4fs_deinit(void); > -int ext4fs_filename_check(char *filename); > +int ext4fs_filename_unlink(char *filename); > int ext4fs_write(const char *fname, unsigned char *buffer, > unsigned long sizebytes); > int ext4_write_file(const char *filename, void *buf, loff_t offset, > loff_t len, Reviewed-by: Lukasz Majewski <l.majewski@samsung.com> -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/3] ext4: propagate error if creation of directory entry fails [not found] <20160814144143.24301-1-stefan.bruens@rwth-aachen.de> 2016-08-14 14:41 ` [U-Boot] [PATCH 1/3 v2] ext4: fix possible crash on directory traversal, ignore deleted entries Stefan Brüns @ 2016-08-14 14:41 ` Stefan Brüns 2016-08-18 14:45 ` Lukasz Majewski 2016-08-14 14:41 ` [U-Boot] [PATCH 3/3] ext4: Do not crash when trying to grow a directory using extents Stefan Brüns 2 siblings, 1 reply; 6+ messages in thread From: Stefan Brüns @ 2016-08-14 14:41 UTC (permalink / raw) To: u-boot In case the dir entry creation failed, ext4fs_write would later overwrite a random inode, as inodeno was never initialized. Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de> --- fs/ext4/ext4_common.c | 12 ++++++------ fs/ext4/ext4_common.h | 2 +- fs/ext4/ext4_write.c | 4 +++- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index 3ecd9a8..b8c37cf 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -344,7 +344,7 @@ static int check_void_in_dentry(struct ext2_dirent *dir, char *filename) return 0; } -void ext4fs_update_parent_dentry(char *filename, int *p_ino, int file_type) +int ext4fs_update_parent_dentry(char *filename, int file_type) { unsigned int *zero_buffer = NULL; char *root_first_block_buffer = NULL; @@ -358,7 +358,7 @@ void ext4fs_update_parent_dentry(char *filename, int *p_ino, int file_type) unsigned int last_entry_dirlen; int sizeof_void_space = 0; int templength = 0; - int inodeno; + int inodeno = -1; int status; struct ext_filesystem *fs = get_fs(); /* directory entry */ @@ -371,13 +371,13 @@ void ext4fs_update_parent_dentry(char *filename, int *p_ino, int file_type) zero_buffer = zalloc(fs->blksz); if (!zero_buffer) { printf("No Memory\n"); - return; + return -1; } root_first_block_buffer = zalloc(fs->blksz); if (!root_first_block_buffer) { free(zero_buffer); printf("No Memory\n"); - return; + return -1; } restart: @@ -496,8 +496,6 @@ restart: temp_dir = temp_dir + sizeof(struct ext2_dirent); memcpy(temp_dir, filename, strlen(filename)); - *p_ino = inodeno; - /* update or write the 1st block of root inode */ if (ext4fs_put_metadata(root_first_block_buffer, first_block_no_of_root)) @@ -506,6 +504,8 @@ restart: fail: free(zero_buffer); free(root_first_block_buffer); + + return inodeno; } static int search_dir(struct ext2_inode *parent_inode, char *dirname) diff --git a/fs/ext4/ext4_common.h b/fs/ext4/ext4_common.h index 370a717..cc9d0c5 100644 --- a/fs/ext4/ext4_common.h +++ b/fs/ext4/ext4_common.h @@ -61,7 +61,7 @@ int ext4fs_iterate_dir(struct ext2fs_node *dir, char *name, uint32_t ext4fs_div_roundup(uint32_t size, uint32_t n); uint16_t ext4fs_checksum_update(unsigned int i); int ext4fs_get_parent_inode_num(const char *dirname, char *dname, int flags); -void ext4fs_update_parent_dentry(char *filename, int *p_ino, int file_type); +int ext4fs_update_parent_dentry(char *filename, int file_type); uint32_t ext4fs_get_new_blk_no(void); int ext4fs_get_new_inode_no(void); void ext4fs_reset_block_bmap(long int blockno, unsigned char *buffer, diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c index f5811aa..4235b95 100644 --- a/fs/ext4/ext4_write.c +++ b/fs/ext4/ext4_write.c @@ -890,7 +890,9 @@ int ext4fs_write(const char *fname, unsigned char *buffer, goto fail; } - ext4fs_update_parent_dentry(filename, &inodeno, FILETYPE_REG); + inodeno = ext4fs_update_parent_dentry(filename, FILETYPE_REG); + if (inodeno == -1) + goto fail; /* prepare file inode */ inode_buffer = zalloc(fs->inodesz); if (!inode_buffer) -- 2.9.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/3] ext4: propagate error if creation of directory entry fails 2016-08-14 14:41 ` [U-Boot] [PATCH 2/3] ext4: propagate error if creation of directory entry fails Stefan Brüns @ 2016-08-18 14:45 ` Lukasz Majewski 0 siblings, 0 replies; 6+ messages in thread From: Lukasz Majewski @ 2016-08-18 14:45 UTC (permalink / raw) To: u-boot Hi Stefan, > In case the dir entry creation failed, ext4fs_write would later > overwrite a random inode, as inodeno was never initialized. > > Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de> > --- > fs/ext4/ext4_common.c | 12 ++++++------ > fs/ext4/ext4_common.h | 2 +- > fs/ext4/ext4_write.c | 4 +++- > 3 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c > index 3ecd9a8..b8c37cf 100644 > --- a/fs/ext4/ext4_common.c > +++ b/fs/ext4/ext4_common.c > @@ -344,7 +344,7 @@ static int check_void_in_dentry(struct > ext2_dirent *dir, char *filename) return 0; > } > > -void ext4fs_update_parent_dentry(char *filename, int *p_ino, int > file_type) +int ext4fs_update_parent_dentry(char *filename, int > file_type) { > unsigned int *zero_buffer = NULL; > char *root_first_block_buffer = NULL; > @@ -358,7 +358,7 @@ void ext4fs_update_parent_dentry(char *filename, > int *p_ino, int file_type) unsigned int last_entry_dirlen; > int sizeof_void_space = 0; > int templength = 0; > - int inodeno; > + int inodeno = -1; > int status; > struct ext_filesystem *fs = get_fs(); > /* directory entry */ > @@ -371,13 +371,13 @@ void ext4fs_update_parent_dentry(char > *filename, int *p_ino, int file_type) zero_buffer = zalloc(fs->blksz); > if (!zero_buffer) { > printf("No Memory\n"); > - return; > + return -1; > } > root_first_block_buffer = zalloc(fs->blksz); > if (!root_first_block_buffer) { > free(zero_buffer); > printf("No Memory\n"); > - return; > + return -1; > } > restart: > > @@ -496,8 +496,6 @@ restart: > temp_dir = temp_dir + sizeof(struct ext2_dirent); > memcpy(temp_dir, filename, strlen(filename)); > > - *p_ino = inodeno; > - > /* update or write the 1st block of root inode */ > if (ext4fs_put_metadata(root_first_block_buffer, > first_block_no_of_root)) > @@ -506,6 +504,8 @@ restart: > fail: > free(zero_buffer); > free(root_first_block_buffer); > + > + return inodeno; > } > > static int search_dir(struct ext2_inode *parent_inode, char *dirname) > diff --git a/fs/ext4/ext4_common.h b/fs/ext4/ext4_common.h > index 370a717..cc9d0c5 100644 > --- a/fs/ext4/ext4_common.h > +++ b/fs/ext4/ext4_common.h > @@ -61,7 +61,7 @@ int ext4fs_iterate_dir(struct ext2fs_node *dir, > char *name, uint32_t ext4fs_div_roundup(uint32_t size, uint32_t n); > uint16_t ext4fs_checksum_update(unsigned int i); > int ext4fs_get_parent_inode_num(const char *dirname, char *dname, > int flags); -void ext4fs_update_parent_dentry(char *filename, int > *p_ino, int file_type); +int ext4fs_update_parent_dentry(char > *filename, int file_type); uint32_t ext4fs_get_new_blk_no(void); > int ext4fs_get_new_inode_no(void); > void ext4fs_reset_block_bmap(long int blockno, unsigned char *buffer, > diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c > index f5811aa..4235b95 100644 > --- a/fs/ext4/ext4_write.c > +++ b/fs/ext4/ext4_write.c > @@ -890,7 +890,9 @@ int ext4fs_write(const char *fname, unsigned char > *buffer, goto fail; > } > > - ext4fs_update_parent_dentry(filename, &inodeno, > FILETYPE_REG); > + inodeno = ext4fs_update_parent_dentry(filename, > FILETYPE_REG); > + if (inodeno == -1) > + goto fail; > /* prepare file inode */ > inode_buffer = zalloc(fs->inodesz); > if (!inode_buffer) Reviewed-by: Lukasz Majewski <l.majewski@samsung.com> -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 3/3] ext4: Do not crash when trying to grow a directory using extents [not found] <20160814144143.24301-1-stefan.bruens@rwth-aachen.de> 2016-08-14 14:41 ` [U-Boot] [PATCH 1/3 v2] ext4: fix possible crash on directory traversal, ignore deleted entries Stefan Brüns 2016-08-14 14:41 ` [U-Boot] [PATCH 2/3] ext4: propagate error if creation of directory entry fails Stefan Brüns @ 2016-08-14 14:41 ` Stefan Brüns 2016-08-18 14:48 ` Lukasz Majewski 2 siblings, 1 reply; 6+ messages in thread From: Stefan Brüns @ 2016-08-14 14:41 UTC (permalink / raw) To: u-boot The following command crashes u-boot: ./sandbox/u-boot -c 'i=0; host bind 0 ./sandbox/test/fs/3GB.ext4.img ; while test $i -lt 200 ; do echo $i; setexpr i $i + 1; ext4write host 0 0 /foobar${i} 0; done' Previously, the code updated the direct_block even for extents, and fortunately crashed before pushing garbage to the disk. Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de> --- fs/ext4/ext4_common.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index b8c37cf..6432104 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -431,8 +431,13 @@ restart: sizeof(struct ext2_dirent) + padding_factor; if ((fs->blksz - totalbytes - last_entry_dirlen) < new_entry_byte_reqd) { - printf("1st Block Full:Allocate new block\n"); + printf("Last Block Full:Allocate new block\n"); + if (le32_to_cpu(g_parent_inode->flags) & + EXT4_EXTENTS_FL) { + printf("Directory uses extents\n"); + goto fail; + } if (direct_blk_idx == INDIRECT_BLOCKS - 1) { printf("Directory exceeds limit\n"); goto fail; -- 2.9.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 3/3] ext4: Do not crash when trying to grow a directory using extents 2016-08-14 14:41 ` [U-Boot] [PATCH 3/3] ext4: Do not crash when trying to grow a directory using extents Stefan Brüns @ 2016-08-18 14:48 ` Lukasz Majewski 0 siblings, 0 replies; 6+ messages in thread From: Lukasz Majewski @ 2016-08-18 14:48 UTC (permalink / raw) To: u-boot Hi Stefan, > The following command crashes u-boot: > ./sandbox/u-boot -c 'i=0; host bind 0 ./sandbox/test/fs/3GB.ext4.img ; > while test $i -lt 200 ; do echo $i; setexpr i $i + 1; > ext4write host 0 0 /foobar${i} 0; done' > > Previously, the code updated the direct_block even for extents, and > fortunately crashed before pushing garbage to the disk. > > Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de> > --- > fs/ext4/ext4_common.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c > index b8c37cf..6432104 100644 > --- a/fs/ext4/ext4_common.c > +++ b/fs/ext4/ext4_common.c > @@ -431,8 +431,13 @@ restart: > sizeof(struct ext2_dirent) + > padding_factor; if ((fs->blksz - totalbytes - last_entry_dirlen) < > new_entry_byte_reqd) { > - printf("1st Block Full:Allocate new > block\n"); > + printf("Last Block Full:Allocate new > block\n"); > + if > (le32_to_cpu(g_parent_inode->flags) & > + EXT4_EXTENTS_FL) { > + printf("Directory uses > extents\n"); > + goto fail; > + } > if (direct_blk_idx == > INDIRECT_BLOCKS - 1) { printf("Directory exceeds limit\n"); > goto fail; Reviewed-by: Lukasz Majewski <l.majewski@samsung.com> -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-08-18 14:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20160814144143.24301-1-stefan.bruens@rwth-aachen.de>
2016-08-14 14:41 ` [U-Boot] [PATCH 1/3 v2] ext4: fix possible crash on directory traversal, ignore deleted entries Stefan Brüns
2016-08-18 14:44 ` Lukasz Majewski
2016-08-14 14:41 ` [U-Boot] [PATCH 2/3] ext4: propagate error if creation of directory entry fails Stefan Brüns
2016-08-18 14:45 ` Lukasz Majewski
2016-08-14 14:41 ` [U-Boot] [PATCH 3/3] ext4: Do not crash when trying to grow a directory using extents Stefan Brüns
2016-08-18 14:48 ` Lukasz Majewski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox