* [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 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 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 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
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
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