[U-Boot] [PATCH v5 01/16] ext4: fix possible crash on directory traversal, ignore deleted entries

[U-Boot] [PATCH v5 01/16] ext4: fix possible crash on directory traversal, ignore deleted entries

[Stefan Brüns]

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>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 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 f3235eb..40203cf 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 3194595..70a3b5b 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;
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.3

[U-Boot] [PATCH v5 02/16] ext4: propagate error if creation of directory entry fails

[Stefan Brüns]

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>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 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 40203cf..5799bee 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -366,7 +366,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;
@@ -380,7 +380,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 */
@@ -393,13 +393,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:
 
@@ -518,8 +518,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))
@@ -528,6 +526,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 70a3b5b..42abd8d 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -907,7 +907,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.3

[U-Boot] [PATCH v5 03/16] ext4: Do not crash when trying to grow a directory using extents

[Stefan Brüns]

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>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 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 5799bee..dff03fe 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -453,8 +453,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.3

[U-Boot] [PATCH v5 04/16] ext4: Scan all directory blocks for space when inserting a new entry

[Stefan Brüns]

Previously, only the last directory block was scanned for available space.
Instead, scan all blocks back to front, and if no sufficient space is
found, eventually append a new block.
Blocks are only appended if the directory does not use extents or the new
block would require insertion of indirect blocks, as the old code does.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 fs/ext4/ext4_common.c | 74 +++++++++++++++++++++------------------------------
 1 file changed, 30 insertions(+), 44 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index dff03fe..062ea93 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -370,14 +370,10 @@ int ext4fs_update_parent_dentry(char *filename, int file_type)
 {
 	unsigned int *zero_buffer = NULL;
 	char *root_first_block_buffer = NULL;
-	int direct_blk_idx;
-	long int root_blknr;
+	int blk_idx;
 	long int first_block_no_of_root = 0;
-	long int previous_blknr = -1;
 	int totalbytes = 0;
-	short int padding_factor = 0;
 	unsigned int new_entry_byte_reqd;
-	unsigned int last_entry_dirlen;
 	int sizeof_void_space = 0;
 	int templength = 0;
 	int inodeno = -1;
@@ -389,6 +385,7 @@ int ext4fs_update_parent_dentry(char *filename, int file_type)
 	uint32_t new_blk_no;
 	uint32_t new_size;
 	uint32_t new_blockcnt;
+	uint32_t directory_blocks;
 
 	zero_buffer = zalloc(fs->blksz);
 	if (!zero_buffer) {
@@ -401,19 +398,18 @@ int ext4fs_update_parent_dentry(char *filename, int file_type)
 		printf("No Memory\n");
 		return -1;
 	}
+	new_entry_byte_reqd = ROUND(strlen(filename) +
+				    sizeof(struct ext2_dirent), 4);
 restart:
+	directory_blocks = le32_to_cpu(g_parent_inode->size) >>
+		LOG2_BLOCK_SIZE(ext4fs_root);
+	blk_idx = directory_blocks - 1;
 
+restart_read:
 	/* read the block no allocated to a file */
-	for (direct_blk_idx = 0; direct_blk_idx < INDIRECT_BLOCKS;
-	     direct_blk_idx++) {
-		root_blknr = read_allocated_block(g_parent_inode,
-						  direct_blk_idx);
-		if (root_blknr == 0) {
-			first_block_no_of_root = previous_blknr;
-			break;
-		}
-		previous_blknr = root_blknr;
-	}
+	first_block_no_of_root = read_allocated_block(g_parent_inode, blk_idx);
+	if (first_block_no_of_root <= 0)
+		goto fail;
 
 	status = ext4fs_devread((lbaint_t)first_block_no_of_root
 				* fs->sect_perblk,
@@ -425,42 +421,33 @@ restart:
 		goto fail;
 	dir = (struct ext2_dirent *)root_first_block_buffer;
 	totalbytes = 0;
+
 	while (le16_to_cpu(dir->direntlen) > 0) {
-		/*
-		 * blocksize-totalbytes because last directory length
-		 * i.e. dir->direntlen is free availble space in the
-		 * block that means  it is a last entry of directory
-		 * entry
-		 */
+		unsigned short used_len = ROUND(dir->namelen +
+		    sizeof(struct ext2_dirent), 4);
 
-		/* traversing the each directory entry */
+		/* last entry of block */
 		if (fs->blksz - totalbytes == le16_to_cpu(dir->direntlen)) {
-			if (strlen(filename) % 4 != 0)
-				padding_factor = 4 - (strlen(filename) % 4);
-
-			new_entry_byte_reqd = strlen(filename) +
-			    sizeof(struct ext2_dirent) + padding_factor;
-			padding_factor = 0;
-			/*
-			 * update last directory entry length to its
-			 * length because we are creating new directory
-			 * entry
-			 */
-			if (dir->namelen % 4 != 0)
-				padding_factor = 4 - (dir->namelen % 4);
 
-			last_entry_dirlen = dir->namelen +
-			    sizeof(struct ext2_dirent) + padding_factor;
-			if ((fs->blksz - totalbytes - last_entry_dirlen) <
-				new_entry_byte_reqd) {
-				printf("Last Block Full:Allocate new block\n");
+			/* check if new entry fits */
+			if ((used_len + new_entry_byte_reqd) <=
+			    le16_to_cpu(dir->direntlen)) {
+				dir->direntlen = cpu_to_le16(used_len);
+				break;
+			} else {
+				if (blk_idx > 0) {
+					printf("Block full, trying previous\n");
+					blk_idx--;
+					goto restart_read;
+				}
+				printf("All blocks full: Allocate new\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) {
+				if (directory_blocks >= INDIRECT_BLOCKS) {
 					printf("Directory exceeds limit\n");
 					goto fail;
 				}
@@ -470,7 +457,8 @@ restart:
 					goto fail;
 				}
 				put_ext4((uint64_t)new_blk_no * fs->blksz, zero_buffer, fs->blksz);
-				g_parent_inode->b.blocks.dir_blocks[direct_blk_idx] =
+				g_parent_inode->b.blocks.
+					dir_blocks[directory_blocks] =
 					cpu_to_le32(new_blk_no);
 
 				new_size = le32_to_cpu(g_parent_inode->size);
@@ -487,8 +475,6 @@ restart:
 					goto fail;
 				goto restart;
 			}
-			dir->direntlen = cpu_to_le16(last_entry_dirlen);
-			break;
 		}
 
 		templength = le16_to_cpu(dir->direntlen);
-- 
2.9.3

[U-Boot] [PATCH v5 05/16] ext4: Avoid corruption of directories with hash tree indexes

[Stefan Brüns]

While directories can be read using the old linear scan method, adding a
new file would require updating the index tree (alternatively, the whole
tree could be removed).

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 fs/ext4/ext4_write.c | 5 +++++
 include/ext4fs.h     | 1 +
 2 files changed, 6 insertions(+)

diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index 42abd8d..50c8415 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -881,6 +881,11 @@ int ext4fs_write(const char *fname, unsigned char *buffer,
 		goto fail;
 	if (ext4fs_iget(parent_inodeno, g_parent_inode))
 		goto fail;
+	/* do not mess up a directory using hash trees */
+	if (le32_to_cpu(g_parent_inode->flags) & EXT4_INDEX_FL) {
+		printf("hash tree directory\n");
+		goto fail;
+	}
 	/* check if the filename is already present in root */
 	existing_file_inodeno = ext4fs_filename_unlink(filename);
 	if (existing_file_inodeno != -1) {
diff --git a/include/ext4fs.h b/include/ext4fs.h
index e3f6216..6e31c73 100644
--- a/include/ext4fs.h
+++ b/include/ext4fs.h
@@ -28,6 +28,7 @@
 #define __EXT4__
 #include <ext_common.h>
 
+#define EXT4_INDEX_FL		0x00001000 /* Inode uses hash tree index */
 #define EXT4_EXTENTS_FL		0x00080000 /* Inode uses extents */
 #define EXT4_EXT_MAGIC			0xf30a
 #define EXT4_FEATURE_RO_COMPAT_GDT_CSUM	0x0010
-- 
2.9.3

[U-Boot] [PATCH v5 06/16] ext4: Scan all directory blocks when looking up an entry

[Stefan Brüns]

Scanning only the direct blocks of the directory file may falsely report
an existing file as nonexisting, and worse can also lead to creation
of a duplicate entry on file creation.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 fs/ext4/ext4_common.c | 84 ++++++++++++++++++++++++---------------------------
 1 file changed, 40 insertions(+), 44 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 062ea93..e8e6c00 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -525,64 +525,57 @@ static int search_dir(struct ext2_inode *parent_inode, char *dirname)
 {
 	int status;
 	int inodeno = 0;
-	int totalbytes;
-	int templength;
-	int direct_blk_idx;
+	int offset;
+	int blk_idx;
 	long int blknr;
-	char *ptr = NULL;
-	unsigned char *block_buffer = NULL;
+	char *block_buffer = NULL;
 	struct ext2_dirent *dir = NULL;
 	struct ext_filesystem *fs = get_fs();
+	uint32_t directory_blocks;
+	char *direntname;
 
-	/* read the block no allocated to a file */
-	for (direct_blk_idx = 0; direct_blk_idx < INDIRECT_BLOCKS;
-		direct_blk_idx++) {
-		blknr = read_allocated_block(parent_inode, direct_blk_idx);
-		if (blknr == 0)
-			goto fail;
+	directory_blocks = le32_to_cpu(parent_inode->size) >>
+		LOG2_BLOCK_SIZE(ext4fs_root);
 
-		/* read the blocks of parent inode */
-		block_buffer = zalloc(fs->blksz);
-		if (!block_buffer)
+	block_buffer = zalloc(fs->blksz);
+	if (!block_buffer)
+		goto fail;
+
+	/* get the block no allocated to a file */
+	for (blk_idx = 0; blk_idx < directory_blocks; blk_idx++) {
+		blknr = read_allocated_block(parent_inode, blk_idx);
+		if (blknr == 0)
 			goto fail;
 
+		/* read the directory block */
 		status = ext4fs_devread((lbaint_t)blknr * fs->sect_perblk,
 					0, fs->blksz, (char *)block_buffer);
 		if (status == 0)
 			goto fail;
 
-		dir = (struct ext2_dirent *)block_buffer;
-		ptr = (char *)dir;
-		totalbytes = 0;
-		while (le16_to_cpu(dir->direntlen) >= 0) {
-			/*
-			 * blocksize-totalbytes because last directory
-			 * length i.e.,*dir->direntlen is free availble
-			 * space in the block that means
-			 * it is a last entry of directory entry
-			 */
-			if (dir->inode && (strlen(dirname) == dir->namelen)) {
-				if (strncmp(dirname, ptr + sizeof(struct ext2_dirent), dir->namelen) == 0) {
-					inodeno = le32_to_cpu(dir->inode);
-					break;
-				}
-			}
+		offset = 0;
+		do {
+			dir = (struct ext2_dirent *)(block_buffer + offset);
+			direntname = (char*)(dir) + sizeof(struct ext2_dirent);
 
-			if (fs->blksz - totalbytes == le16_to_cpu(dir->direntlen))
+			int direntlen = le16_to_cpu(dir->direntlen);
+			if (direntlen < sizeof(struct ext2_dirent))
 				break;
 
-			/* traversing the each directory entry */
-			templength = le16_to_cpu(dir->direntlen);
-			totalbytes = totalbytes + templength;
-			dir = (struct ext2_dirent *)((char *)dir + templength);
-			ptr = (char *)dir;
-		}
+			if (dir->inode && (strlen(dirname) == dir->namelen) &&
+			    (strncmp(dirname, direntname, dir->namelen) == 0)) {
+				inodeno = le32_to_cpu(dir->inode);
+				break;
+			}
+
+			offset += direntlen;
 
-		free(block_buffer);
-		block_buffer = NULL;
+		} while (offset < fs->blksz);
 
-		if (inodeno > 0)
+		if (inodeno > 0) {
+			free(block_buffer);
 			return inodeno;
+		}
 	}
 
 fail:
@@ -834,14 +827,17 @@ fail:
 
 int ext4fs_filename_unlink(char *filename)
 {
-	short direct_blk_idx = 0;
+	int blk_idx;
 	long int blknr = -1;
 	int inodeno = -1;
+	uint32_t directory_blocks;
+
+	directory_blocks = le32_to_cpu(g_parent_inode->size) >>
+		LOG2_BLOCK_SIZE(ext4fs_root);
 
 	/* read the block no allocated to a file */
-	for (direct_blk_idx = 0; direct_blk_idx < INDIRECT_BLOCKS;
-		direct_blk_idx++) {
-		blknr = read_allocated_block(g_parent_inode, direct_blk_idx);
+	for (blk_idx = 0; blk_idx < directory_blocks; blk_idx++) {
+		blknr = read_allocated_block(g_parent_inode, blk_idx);
 		if (blknr == 0)
 			break;
 		inodeno = unlink_filename(filename, blknr);
-- 
2.9.3

[U-Boot] [PATCH v5 07/16] ext4: Only update number of of unused inodes if GDT_CSUM feature is set

[Stefan Brüns]

e2fsck warns about "Group descriptor 0 marked uninitialized without
feature set."
The bg_itable_unused field is only defined if FEATURE_RO_COMPAT_GDT_CSUM
is set, and should be set (kept) zero otherwise.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 fs/ext4/ext4_common.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index e8e6c00..e58ba36 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -988,12 +988,13 @@ int ext4fs_get_new_inode_no(void)
 	if (!journal_buffer || !zero_buffer)
 		goto fail;
 	struct ext2_block_group *bgd = (struct ext2_block_group *)fs->gdtable;
+	int has_gdt_chksum = le32_to_cpu(fs->sb->feature_ro_compat) &
+		EXT4_FEATURE_RO_COMPAT_GDT_CSUM ? 1 : 0;
 
 	if (fs->first_pass_ibmap == 0) {
 		for (i = 0; i < fs->no_blkgrp; i++) {
 			if (bgd[i].free_inodes) {
-				if (bgd[i].bg_itable_unused !=
-						bgd[i].free_inodes)
+				if (has_gdt_chksum)
 					bgd[i].bg_itable_unused =
 						bgd[i].free_inodes;
 				if (le16_to_cpu(bgd[i].bg_flags) & EXT4_BG_INODE_UNINIT) {
@@ -1014,7 +1015,8 @@ int ext4fs_get_new_inode_no(void)
 							(i * inodes_per_grp);
 				fs->first_pass_ibmap++;
 				ext4fs_bg_free_inodes_dec(&bgd[i]);
-				ext4fs_bg_itable_unused_dec(&bgd[i]);
+				if (has_gdt_chksum)
+					ext4fs_bg_itable_unused_dec(&bgd[i]);
 				ext4fs_sb_free_inodes_dec(fs->sb);
 				status = ext4fs_devread(
 							(lbaint_t)le32_to_cpu(bgd[i].inode_id) *
@@ -1069,12 +1071,10 @@ restart:
 				goto fail;
 			prev_inode_bitmap_index = ibmap_idx;
 		}
-		if (bgd[ibmap_idx].bg_itable_unused !=
-				bgd[ibmap_idx].free_inodes)
+		ext4fs_bg_free_inodes_dec(&bgd[ibmap_idx]);
+		if (has_gdt_chksum)
 			bgd[ibmap_idx].bg_itable_unused =
 					bgd[ibmap_idx].free_inodes;
-		ext4fs_bg_free_inodes_dec(&bgd[ibmap_idx]);
-		ext4fs_bg_itable_unused_dec(&bgd[ibmap_idx]);
 		ext4fs_sb_free_inodes_dec(fs->sb);
 		goto success;
 	}
-- 
2.9.3

[U-Boot] [PATCH v5 08/16] ext4: Do not clear zalloc'ed buffers a second time

[Stefan Brüns]

zero_buffer is never written, thus clearing it is pointless.
journal_buffer is completely initialized by ext4fs_devread (or in case
of failure, not used).

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 fs/ext4/ext4_common.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index e58ba36..539d622 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -929,7 +929,6 @@ restart:
 
 		if (le16_to_cpu(bgd[bg_idx].bg_flags) & EXT4_BG_BLOCK_UNINIT) {
 			uint16_t new_flags;
-			memset(zero_buffer, '\0', fs->blksz);
 			put_ext4((uint64_t)le32_to_cpu(bgd[bg_idx].block_id) * fs->blksz,
 				 zero_buffer, fs->blksz);
 			memcpy(fs->blk_bmaps[bg_idx], zero_buffer, fs->blksz);
@@ -946,7 +945,6 @@ restart:
 
 		/* journal backup */
 		if (prev_bg_bitmap_index != bg_idx) {
-			memset(journal_buffer, '\0', fs->blksz);
 			status = ext4fs_devread(
 						(lbaint_t)le32_to_cpu(bgd[bg_idx].block_id)
 						* fs->sect_perblk,
@@ -1040,7 +1038,6 @@ restart:
 		ibmap_idx = fs->curr_inode_no / inodes_per_grp;
 		if (le16_to_cpu(bgd[ibmap_idx].bg_flags) & EXT4_BG_INODE_UNINIT) {
 			int new_flags;
-			memset(zero_buffer, '\0', fs->blksz);
 			put_ext4((uint64_t)le32_to_cpu(bgd[ibmap_idx].inode_id) * fs->blksz,
 				 zero_buffer, fs->blksz);
 			new_flags = le16_to_cpu(bgd[ibmap_idx].bg_flags) & ~EXT4_BG_INODE_UNINIT;
-- 
2.9.3

[U-Boot] [PATCH v5 09/16] ext4: After completely filled group, scan next group from the beginning

[Stefan Brüns]

The last free block of a block group may be in its middle. After it has
been allocated, the next block group should be scanned from its beginning.

The following command triggers the bad behaviour (on a blocksize 1024 fs):

./sandbox/u-boot -c 'i=0; host bind 0 ./disk.raw ;
	while test $i -lt 260 ; do echo $i; setexpr i $i + 1;
		ext4write host 0:2 0 /X${i} 0x1450; done ;
	ext4write host 0:2 0 /X240 0x2000 ; '

When 'X240' is extended from 5200 byte to 8192 byte, the new blocks should
start from the first free block (8811), but it uses the blocks 8098-8103
and 16296-16297 -- 8103 + 1 + 8192 = 16296. This can be shown with
debugfs, commands 'ffb' and 'stat X240'.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 fs/ext4/ext4_common.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 539d622..8fc7559 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -903,8 +903,8 @@ uint32_t ext4fs_get_new_blk_no(void)
 
 		goto fail;
 	} else {
-restart:
 		fs->curr_blkno++;
+restart:
 		/* get the blockbitmap index respective to blockno */
 		bg_idx = fs->curr_blkno / blk_per_grp;
 		if (fs->blksz == 1024) {
@@ -922,8 +922,9 @@ restart:
 
 		if (bgd[bg_idx].free_blocks == 0) {
 			debug("block group %u is full. Skipping\n", bg_idx);
-			fs->curr_blkno = fs->curr_blkno + blk_per_grp;
-			fs->curr_blkno--;
+			fs->curr_blkno = (bg_idx + 1) * blk_per_grp;
+			if (fs->blksz == 1024)
+				fs->curr_blkno += 1;
 			goto restart;
 		}
 
@@ -940,6 +941,7 @@ restart:
 				   bg_idx) != 0) {
 			debug("going for restart for the block no %ld %u\n",
 			      fs->curr_blkno, bg_idx);
+			fs->curr_blkno++;
 			goto restart;
 		}
 
-- 
2.9.3

[U-Boot] [PATCH v5 10/16] ext4: Avoid out-of-bounds access of block bitmap

[Stefan Brüns]

If the blocksize is 1024, count is initialized with 1. Incrementing count
by 8 will never match (count == fs->blksz * 8), and ptr may be
incremented beyond the buffer end if the bitmap is filled. Add the
startblock offset after the loop.

Remove the second loop, as only the first iteration will be done.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 fs/ext4/ext4_common.c | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 8fc7559..72fc4af 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -163,18 +163,12 @@ static int _get_new_inode_no(unsigned char *buffer)
 
 static int _get_new_blk_no(unsigned char *buffer)
 {
-	unsigned char input;
-	int operand, status;
+	int operand;
 	int count = 0;
-	int j = 0;
+	int i;
 	unsigned char *ptr = buffer;
 	struct ext_filesystem *fs = get_fs();
 
-	if (fs->blksz != 1024)
-		count = 0;
-	else
-		count = 1;
-
 	while (*ptr == 255) {
 		ptr++;
 		count += 8;
@@ -182,21 +176,17 @@ static int _get_new_blk_no(unsigned char *buffer)
 			return -1;
 	}
 
-	for (j = 0; j < fs->blksz; j++) {
-		input = *ptr;
-		int i = 0;
-		while (i <= 7) {
-			operand = 1 << i;
-			status = input & operand;
-			if (status) {
-				i++;
-				count++;
-			} else {
-				*ptr |= operand;
-				return count;
-			}
+	if (fs->blksz == 1024)
+		count += 1;
+
+	for (i = 0; i <= 7; i++) {
+		operand = 1 << i;
+		if (*ptr & operand) {
+			count++;
+		} else {
+			*ptr |= operand;
+			return count;
 		}
-		ptr = ptr + 1;
 	}
 
 	return -1;
-- 
2.9.3

[U-Boot] [PATCH v5 11/16] ext4: Fix memory leak in case of failure

[Stefan Brüns]

temp_ptr should always be freed, even if the function is left via
goto fail.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 fs/ext4/ext4_write.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index 50c8415..5e208ef 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -974,7 +974,6 @@ int ext4fs_write(const char *fname, unsigned char *buffer,
 			sizeof(struct ext2_inode));
 		if (ext4fs_put_metadata(temp_ptr, parent_itable_blkno))
 			goto fail;
-		free(temp_ptr);
 	} else {
 		/*
 		 * If parent and child fall in same inode table block
@@ -985,7 +984,6 @@ int ext4fs_write(const char *fname, unsigned char *buffer,
 		gd_index--;
 		if (ext4fs_put_metadata(temp_ptr, itable_blkno))
 			goto fail;
-		free(temp_ptr);
 	}
 	ext4fs_update();
 	ext4fs_deinit();
@@ -996,6 +994,7 @@ int ext4fs_write(const char *fname, unsigned char *buffer,
 	fs->curr_inode_no = 0;
 	free(inode_buffer);
 	free(g_parent_inode);
+	free(temp_ptr);
 	g_parent_inode = NULL;
 
 	return 0;
@@ -1003,6 +1002,7 @@ fail:
 	ext4fs_deinit();
 	free(inode_buffer);
 	free(g_parent_inode);
+	free(temp_ptr);
 	g_parent_inode = NULL;
 
 	return -1;
-- 
2.9.3

[U-Boot] [PATCH v5 12/16] ext4: Use correct value for inode size even on revision 0 filesystems

[Stefan Brüns]

fs->inodesz is already correctly (i.e. dependent on fs revision)
initialized in ext4fs_mount.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 fs/ext4/ext4_write.c | 1 -
 include/ext_common.h | 2 --
 2 files changed, 3 deletions(-)

diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index 5e208ef..81a750b 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -609,7 +609,6 @@ int ext4fs_init(void)
 
 	/* populate fs */
 	fs->blksz = EXT2_BLOCK_SIZE(ext4fs_root);
-	fs->inodesz = INODE_SIZE_FILESYSTEM(ext4fs_root);
 	fs->sect_perblk = fs->blksz >> fs->dev_desc->log2blksz;
 
 	/* get the superblock */
diff --git a/include/ext_common.h b/include/ext_common.h
index 4cd2aa7..25216ca 100644
--- a/include/ext_common.h
+++ b/include/ext_common.h
@@ -52,8 +52,6 @@
 #define LOG2_BLOCK_SIZE(data)	   (le32_to_cpu		   \
 				    (data->sblock.log2_block_size) \
 				    + EXT2_MIN_BLOCK_LOG_SIZE)
-#define INODE_SIZE_FILESYSTEM(data)	(le16_to_cpu \
-			(data->sblock.inode_size))
 
 #define EXT2_FT_DIR	2
 #define SUCCESS	1
-- 
2.9.3

[U-Boot] [PATCH v5 13/16] ext4: initialize full inode for inodes bigger than 128 bytes

[Stefan Brüns]

Make sure the the extra_isize field (offset 128) is initialized to 0, to
mark any extra data as invalid.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
---
 fs/ext4/ext4_write.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index 81a750b..5b518ed 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -560,7 +560,7 @@ static int ext4fs_delete_file(int inodeno)
 
 	read_buffer = read_buffer + blkoff;
 	inode_buffer = (struct ext2_inode *)read_buffer;
-	memset(inode_buffer, '\0', sizeof(struct ext2_inode));
+	memset(inode_buffer, '\0', fs->inodesz);
 
 	/* write the inode to original position in inode table */
 	if (ext4fs_put_metadata(start_block_address, blkno))
@@ -866,7 +866,7 @@ int ext4fs_write(const char *fname, unsigned char *buffer,
 	ALLOC_CACHE_ALIGN_BUFFER(char, filename, 256);
 	memset(filename, 0x00, 256);
 
-	g_parent_inode = zalloc(sizeof(struct ext2_inode));
+	g_parent_inode = zalloc(fs->inodesz);
 	if (!g_parent_inode)
 		goto fail;
 
@@ -969,8 +969,7 @@ int ext4fs_write(const char *fname, unsigned char *buffer,
 		if (ext4fs_log_journal(temp_ptr, parent_itable_blkno))
 			goto fail;
 
-		memcpy(temp_ptr + blkoff, g_parent_inode,
-			sizeof(struct ext2_inode));
+		memcpy(temp_ptr + blkoff, g_parent_inode, fs->inodesz);
 		if (ext4fs_put_metadata(temp_ptr, parent_itable_blkno))
 			goto fail;
 	} else {
@@ -978,8 +977,7 @@ int ext4fs_write(const char *fname, unsigned char *buffer,
 		 * If parent and child fall in same inode table block
 		 * both should be kept in 1 buffer
 		 */
-		memcpy(temp_ptr + blkoff, g_parent_inode,
-		       sizeof(struct ext2_inode));
+		memcpy(temp_ptr + blkoff, g_parent_inode, fs->inodesz);
 		gd_index--;
 		if (ext4fs_put_metadata(temp_ptr, itable_blkno))
 			goto fail;
-- 
2.9.3

[U-Boot] [PATCH v5 14/16] ext4: remove duplicated block release code for extents

[Stefan Brüns]

The data blocks are identical for files using traditional direct/indirect
block allocation scheme and extent trees, thus this code part can be
common. Only the code to deallocate the indirect blocks to record the
used blocks has to be seperate, respectively the code to release extent
tree index blocks.

Actually the code to release the extent tree index blocks is still missing,
but at least add a FIXME at the appropriate place.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 fs/ext4/ext4_write.c | 110 ++++++++++++++++-----------------------------------
 1 file changed, 33 insertions(+), 77 deletions(-)

diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index 5b518ed..20005f5 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -447,92 +447,48 @@ static int ext4fs_delete_file(int inodeno)
 		no_blocks++;
 
 	if (le32_to_cpu(inode.flags) & EXT4_EXTENTS_FL) {
-		struct ext2fs_node *node_inode =
-		    zalloc(sizeof(struct ext2fs_node));
-		if (!node_inode)
-			goto fail;
-		node_inode->data = ext4fs_root;
-		node_inode->ino = inodeno;
-		node_inode->inode_read = 0;
-		memcpy(&(node_inode->inode), &inode, sizeof(struct ext2_inode));
-
-		for (i = 0; i < no_blocks; i++) {
-			blknr = read_allocated_block(&(node_inode->inode), i);
-			bg_idx = blknr / blk_per_grp;
-			if (fs->blksz == 1024) {
-				remainder = blknr % blk_per_grp;
-				if (!remainder)
-					bg_idx--;
-			}
-			ext4fs_reset_block_bmap(blknr, fs->blk_bmaps[bg_idx],
-						bg_idx);
-			debug("EXT4_EXTENTS Block releasing %ld: %d\n",
-			      blknr, bg_idx);
-
-			ext4fs_bg_free_blocks_inc(&bgd[bg_idx]);
-			ext4fs_sb_free_blocks_inc(fs->sb);
-
-			/* journal backup */
-			if (prev_bg_bmap_idx != bg_idx) {
-				status =
-				    ext4fs_devread(
-						   (lbaint_t)le32_to_cpu(bgd[bg_idx].block_id) *
-						   fs->sect_perblk, 0,
-						   fs->blksz, journal_buffer);
-				if (status == 0)
-					goto fail;
-				if (ext4fs_log_journal(journal_buffer,
-							le32_to_cpu(bgd[bg_idx].block_id)))
-					goto fail;
-				prev_bg_bmap_idx = bg_idx;
-			}
-		}
-		if (node_inode) {
-			free(node_inode);
-			node_inode = NULL;
-		}
+		/* FIXME delete extent index blocks, i.e. eh_depth >= 1 */
+		struct ext4_extent_header *eh =
+			(struct ext4_extent_header *)
+				inode.b.blocks.dir_blocks;
+		debug("del: dep=%d entries=%d\n", eh->eh_depth, eh->eh_entries);
 	} else {
-
 		delete_single_indirect_block(&inode);
 		delete_double_indirect_block(&inode);
 		delete_triple_indirect_block(&inode);
+	}
 
-		/* read the block no allocated to a file */
-		no_blocks = le32_to_cpu(inode.size) / fs->blksz;
-		if (le32_to_cpu(inode.size) % fs->blksz)
-			no_blocks++;
-		for (i = 0; i < no_blocks; i++) {
-			blknr = read_allocated_block(&inode, i);
-			bg_idx = blknr / blk_per_grp;
-			if (fs->blksz == 1024) {
-				remainder = blknr % blk_per_grp;
-				if (!remainder)
-					bg_idx--;
-			}
-			ext4fs_reset_block_bmap(blknr, fs->blk_bmaps[bg_idx],
-						bg_idx);
-			debug("ActualB releasing %ld: %d\n", blknr, bg_idx);
+	/* release data blocks */
+	for (i = 0; i < no_blocks; i++) {
+		blknr = read_allocated_block(&inode, i);
+		bg_idx = blknr / blk_per_grp;
+		if (fs->blksz == 1024) {
+			remainder = blknr % blk_per_grp;
+			if (!remainder)
+				bg_idx--;
+		}
+		ext4fs_reset_block_bmap(blknr, fs->blk_bmaps[bg_idx],
+					bg_idx);
+		debug("EXT4 Block releasing %ld: %d\n", blknr, bg_idx);
 
-			ext4fs_bg_free_blocks_inc(&bgd[bg_idx]);
-			ext4fs_sb_free_blocks_inc(fs->sb);
-			/* journal backup */
-			if (prev_bg_bmap_idx != bg_idx) {
-				memset(journal_buffer, '\0', fs->blksz);
-				status = ext4fs_devread(
-							(lbaint_t)le32_to_cpu(bgd[bg_idx].block_id)
-							* fs->sect_perblk,
-							0, fs->blksz,
-							journal_buffer);
-				if (status == 0)
-					goto fail;
-				if (ext4fs_log_journal(journal_buffer,
-						le32_to_cpu(bgd[bg_idx].block_id)))
-					goto fail;
-				prev_bg_bmap_idx = bg_idx;
-			}
+		ext4fs_bg_free_blocks_inc(&bgd[bg_idx]);
+		ext4fs_sb_free_blocks_inc(fs->sb);
+		/* journal backup */
+		if (prev_bg_bmap_idx != bg_idx) {
+			uint32_t bgd_blknr = le32_to_cpu(bgd[bg_idx].block_id);
+			status = ext4fs_devread((lbaint_t)bgd_blknr *
+						fs->sect_perblk,
+						0, fs->blksz,
+						journal_buffer);
+			if (status == 0)
+				goto fail;
+			if (ext4fs_log_journal(journal_buffer, bgd_blknr))
+				goto fail;
+			prev_bg_bmap_idx = bg_idx;
 		}
 	}
 
+	/* release inode */
 	/* from the inode no to blockno */
 	inodes_per_block = fs->blksz / fs->inodesz;
 	ibmap_idx = inodeno / inode_per_grp;
-- 
2.9.3

[U-Boot] [PATCH v5 15/16] ext4: Correct block number handling, empty block vs. error code

[Stefan Brüns]

read_allocated block may return block number 0, which is just an indicator
a chunk of the file is not backed by a block, i.e. it is sparse.

During file deletions, just continue with the next logical block, for other
operations treat blocknumber <= 0 as an error.

For writes, blocknumber 0 should never happen, as U-Boot always allocates
blocks for the whole file.  Reading already handles this correctly, i.e. the
read buffer is 0-fillled.

Not treating block 0 as sparse block leads to FS corruption, e.g.
	./sandbox/u-boot -c 'host bind 0 ./sandbox/test/fs/3GB.ext4.img ;
		ext4write host 0 0 /2.5GB.file 1 '
The 2.5GB.file from the fs test is actually a sparse file.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 fs/ext4/ext4_common.c |  6 +++---
 fs/ext4/ext4_write.c  | 11 ++++++++++-
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 72fc4af..a78b0b8 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -534,7 +534,7 @@ static int search_dir(struct ext2_inode *parent_inode, char *dirname)
 	/* get the block no allocated to a file */
 	for (blk_idx = 0; blk_idx < directory_blocks; blk_idx++) {
 		blknr = read_allocated_block(parent_inode, blk_idx);
-		if (blknr == 0)
+		if (blknr <= 0)
 			goto fail;
 
 		/* read the directory block */
@@ -828,7 +828,7 @@ int ext4fs_filename_unlink(char *filename)
 	/* read the block no allocated to a file */
 	for (blk_idx = 0; blk_idx < directory_blocks; blk_idx++) {
 		blknr = read_allocated_block(g_parent_inode, blk_idx);
-		if (blknr == 0)
+		if (blknr <= 0)
 			break;
 		inodeno = unlink_filename(filename, blknr);
 		if (inodeno != -1)
@@ -1590,7 +1590,7 @@ long int read_allocated_block(struct ext2_inode *inode, int fileblock)
 			if (status == 0) {
 				printf("** SI ext2fs read block (indir 1)"
 					"failed. **\n");
-				return 0;
+				return -1;
 			}
 			ext4fs_indir1_blkno =
 				le32_to_cpu(inode->b.blocks.
diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index 20005f5..b3ea07b 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -461,6 +461,10 @@ static int ext4fs_delete_file(int inodeno)
 	/* release data blocks */
 	for (i = 0; i < no_blocks; i++) {
 		blknr = read_allocated_block(&inode, i);
+		if (blknr == 0)
+			continue;
+		if (blknr < 0)
+			goto fail;
 		bg_idx = blknr / blk_per_grp;
 		if (fs->blksz == 1024) {
 			remainder = blknr % blk_per_grp;
@@ -718,6 +722,10 @@ void ext4fs_deinit(void)
 	fs->curr_blkno = 0;
 }
 
+/*
+ * Write data to filesystem blocks. Uses same optimization for
+ * contigous sectors as ext4fs_read_file
+ */
 static int ext4fs_write_file(struct ext2_inode *file_inode,
 			     int pos, unsigned int len, char *buf)
 {
@@ -744,7 +752,7 @@ static int ext4fs_write_file(struct ext2_inode *file_inode,
 		int blockend = fs->blksz;
 		int skipfirst = 0;
 		blknr = read_allocated_block(file_inode, i);
-		if (blknr < 0)
+		if (blknr <= 0)
 			return -1;
 
 		blknr = blknr << log2_fs_blocksize;
@@ -910,6 +918,7 @@ int ext4fs_write(const char *fname, unsigned char *buffer,
 	/* copy the file content into data blocks */
 	if (ext4fs_write_file(file_inode, 0, sizebytes, (char *)buffer) == -1) {
 		printf("Error in copying content\n");
+		/* FIXME: Deallocate data blocks */
 		goto fail;
 	}
 	ibmap_idx = parent_inodeno / le32_to_cpu(ext4fs_root->sblock.inodes_per_group);
-- 
2.9.3

[U-Boot] [PATCH v5 16/16] ext4: Fix memory leak of journal buffer if block is updated multiple times

[Stefan Brüns]

If the same block is updated multiple times in a row during a single
file system operation, gd_index is decremented to use the same journal
entry again. Avoid loosing the already allocated buffer.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 fs/ext4/ext4_journal.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4_journal.c b/fs/ext4/ext4_journal.c
index cf14049..5a25be4 100644
--- a/fs/ext4/ext4_journal.c
+++ b/fs/ext4/ext4_journal.c
@@ -190,7 +190,11 @@ int ext4fs_put_metadata(char *metadata_buffer, uint32_t blknr)
 		printf("Invalid input arguments %s\n", __func__);
 		return -EINVAL;
 	}
-	dirty_block_ptr[gd_index]->buf = zalloc(fs->blksz);
+	if (dirty_block_ptr[gd_index]->buf)
+		assert(dirty_block_ptr[gd_index]->blknr == blknr);
+	else
+		dirty_block_ptr[gd_index]->buf = zalloc(fs->blksz);
+
 	if (!dirty_block_ptr[gd_index]->buf)
 		return -ENOMEM;
 	memcpy(dirty_block_ptr[gd_index]->buf, metadata_buffer, fs->blksz);
-- 
2.9.3

[U-Boot] [U-Boot, v5, 01/16] ext4: fix possible crash on directory traversal, ignore deleted entries

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:41AM +0200, Stefan Br?ns wrote:

> 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>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/43b95355/attachment-0001.sig>

[U-Boot] [U-Boot, v5, 02/16] ext4: propagate error if creation of directory entry fails

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:42AM +0200, Stefan Br?ns wrote:

> 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>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/a2f789cc/attachment-0001.sig>

[U-Boot] [U-Boot, v5, 03/16] ext4: Do not crash when trying to grow a directory using extents

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:43AM +0200, Stefan Br?ns wrote:

> 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>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/c54b95ae/attachment-0001.sig>

[U-Boot] [U-Boot, v5, 04/16] ext4: Scan all directory blocks for space when inserting a new entry

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:44AM +0200, Stefan Br?ns wrote:

> Previously, only the last directory block was scanned for available space.
> Instead, scan all blocks back to front, and if no sufficient space is
> found, eventually append a new block.
> Blocks are only appended if the directory does not use extents or the new
> block would require insertion of indirect blocks, as the old code does.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/0bf25c7b/attachment-0001.sig>

[U-Boot] [U-Boot, v5, 05/16] ext4: Avoid corruption of directories with hash tree indexes

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:45AM +0200, Stefan Br?ns wrote:

> While directories can be read using the old linear scan method, adding a
> new file would require updating the index tree (alternatively, the whole
> tree could be removed).
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/20565260/attachment-0001.sig>

[U-Boot] [U-Boot, v5, 06/16] ext4: Scan all directory blocks when looking up an entry

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:46AM +0200, Stefan Br?ns wrote:

> Scanning only the direct blocks of the directory file may falsely report
> an existing file as nonexisting, and worse can also lead to creation
> of a duplicate entry on file creation.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/e5289d4f/attachment-0001.sig>

[U-Boot] [U-Boot, v5, 07/16] ext4: Only update number of of unused inodes if GDT_CSUM feature is set

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:47AM +0200, Stefan Br?ns wrote:

> e2fsck warns about "Group descriptor 0 marked uninitialized without
> feature set."
> The bg_itable_unused field is only defined if FEATURE_RO_COMPAT_GDT_CSUM
> is set, and should be set (kept) zero otherwise.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/807a8707/attachment-0001.sig>

[U-Boot] [U-Boot, v5, 08/16] ext4: Do not clear zalloc'ed buffers a second time

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:48AM +0200, Stefan Br?ns wrote:

> zero_buffer is never written, thus clearing it is pointless.
> journal_buffer is completely initialized by ext4fs_devread (or in case
> of failure, not used).
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/e6572b95/attachment.sig>

[U-Boot] [U-Boot, v5, 09/16] ext4: After completely filled group, scan next group from the beginning

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:49AM +0200, Stefan Br?ns wrote:

> The last free block of a block group may be in its middle. After it has
> been allocated, the next block group should be scanned from its beginning.
> 
> The following command triggers the bad behaviour (on a blocksize 1024 fs):
> 
> ./sandbox/u-boot -c 'i=0; host bind 0 ./disk.raw ;
> 	while test $i -lt 260 ; do echo $i; setexpr i $i + 1;
> 		ext4write host 0:2 0 /X${i} 0x1450; done ;
> 	ext4write host 0:2 0 /X240 0x2000 ; '
> 
> When 'X240' is extended from 5200 byte to 8192 byte, the new blocks should
> start from the first free block (8811), but it uses the blocks 8098-8103
> and 16296-16297 -- 8103 + 1 + 8192 = 16296. This can be shown with
> debugfs, commands 'ffb' and 'stat X240'.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/825e8086/attachment.sig>

[U-Boot] [U-Boot, v5, 10/16] ext4: Avoid out-of-bounds access of block bitmap

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:50AM +0200, Stefan Br?ns wrote:

> If the blocksize is 1024, count is initialized with 1. Incrementing count
> by 8 will never match (count == fs->blksz * 8), and ptr may be
> incremented beyond the buffer end if the bitmap is filled. Add the
> startblock offset after the loop.
> 
> Remove the second loop, as only the first iteration will be done.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/5deefb80/attachment.sig>

[U-Boot] [U-Boot, v5, 11/16] ext4: Fix memory leak in case of failure

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:51AM +0200, Stefan Br?ns wrote:

> temp_ptr should always be freed, even if the function is left via
> goto fail.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/202fb4f4/attachment.sig>

[U-Boot] [U-Boot, v5, 12/16] ext4: Use correct value for inode size even on revision 0 filesystems

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:52AM +0200, Stefan Br?ns wrote:

> fs->inodesz is already correctly (i.e. dependent on fs revision)
> initialized in ext4fs_mount.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/09b76b43/attachment.sig>

[U-Boot] [U-Boot, v5, 13/16] ext4: initialize full inode for inodes bigger than 128 bytes

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:53AM +0200, Stefan Br?ns wrote:

> Make sure the the extra_isize field (offset 128) is initialized to 0, to
> mark any extra data as invalid.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/4bb028da/attachment.sig>

[U-Boot] [U-Boot, v5, 14/16] ext4: remove duplicated block release code for extents

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:54AM +0200, Stefan Br?ns wrote:

> The data blocks are identical for files using traditional direct/indirect
> block allocation scheme and extent trees, thus this code part can be
> common. Only the code to deallocate the indirect blocks to record the
> used blocks has to be seperate, respectively the code to release extent
> tree index blocks.
> 
> Actually the code to release the extent tree index blocks is still missing,
> but at least add a FIXME at the appropriate place.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/b2dee734/attachment.sig>

[U-Boot] [U-Boot, v5, 15/16] ext4: Correct block number handling, empty block vs. error code

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:55AM +0200, Stefan Br?ns wrote:

> read_allocated block may return block number 0, which is just an indicator
> a chunk of the file is not backed by a block, i.e. it is sparse.
> 
> During file deletions, just continue with the next logical block, for other
> operations treat blocknumber <= 0 as an error.
> 
> For writes, blocknumber 0 should never happen, as U-Boot always allocates
> blocks for the whole file.  Reading already handles this correctly, i.e. the
> read buffer is 0-fillled.
> 
> Not treating block 0 as sparse block leads to FS corruption, e.g.
> 	./sandbox/u-boot -c 'host bind 0 ./sandbox/test/fs/3GB.ext4.img ;
> 		ext4write host 0 0 /2.5GB.file 1 '
> The 2.5GB.file from the fs test is actually a sparse file.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/6ee2ea98/attachment.sig>

[U-Boot] [U-Boot, v5, 16/16] ext4: Fix memory leak of journal buffer if block is updated multiple times

[Tom Rini]

On Tue, Sep 06, 2016 at 04:36:56AM +0200, Stefan Br?ns wrote:

> If the same block is updated multiple times in a row during a single
> file system operation, gd_index is decremented to use the same journal
> entry again. Avoid loosing the already allocated buffer.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/3bfaa69f/attachment.sig>