[U-Boot] [PATCH v2 2/2] fs:ext4:write:fix: Reinitialize global variables after updating a file

Lukasz Majewski l.majewski at samsung.com
Tue May 6 09:36:05 CEST 2014


This bug shows up when file stored on the ext4 file system is updated.

The ext4fs_delete_file() is responsible for deleting file's (e.g. uImage)
data.
However some global data (especially ext4fs_indir2_block), which is used
during file deletion are left unchanged.

The ext4fs_indir2_block pointer stores reference to old ext4 double
indirect allocated blocks. When it is unchanged, after file deletion,
ext4fs_write_file() uses the same pointer (since it is already initialized
- i.e. not NULL) to return number of blocks to write. This trunks larger
file when previous one was smaller.

Lets consider following scenario:

1. Flash target with ext4 formatted boot.img (which has uImage [*] on itself)
2. Developer wants to upload their custom uImage [**]
	- When new uImage [**] is smaller than the [*] - everything works
	correctly - we are able to store the whole smaller file with corrupted
	ext4fs_indir2_block pointer
	- When new uImage [**] is larger than the [*] - theCRC is corrupted,
	since truncation on data stored at eMMC was done.
3. When uImage CRC error appears, then reboot and LTHOR/DFU reflashing causes
	proper setting of ext4fs_indir2_block() and after that uImage[**]
	is successfully stored (correct uImage [*] metadata is stored at an
	eMMC on the first flashing).

Due to above the bug was very difficult to reproduce.
This patch sets default values for all ext4fs_indir* pointers/variables.

Signed-off-by: Lukasz Majewski <l.majewski at samsung.com>
---
Changes for v2:
- Add comment describing the purpose of ext4fs_reinit_global() function
---
 fs/ext4/ext4_common.c |   35 ++++++++++++++++++++++++++---------
 fs/ext4/ext4_write.c  |    1 +
 include/ext4fs.h      |    1 +
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 62e2e80..1c11721 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -1841,16 +1841,20 @@ long int read_allocated_block(struct ext2_inode *inode, int fileblock)
 	return blknr;
 }
 
-void ext4fs_close(void)
+/**
+ * ext4fs_reinit_global() - Reinitialize values of ext4 write implementation's
+ *			    global pointers
+ *
+ * This function assures that for a file with the same name but different size
+ * the sequential store on the ext4 filesystem will be correct.
+ *
+ * In this function the global data, responsible for internal representation
+ * of the ext4 data are initialized to the reset state. Without this, during
+ * replacement of the smaller file with the bigger truncation of new file was
+ * performed.
+ */
+void ext4fs_reinit_global(void)
 {
-	if ((ext4fs_file != NULL) && (ext4fs_root != NULL)) {
-		ext4fs_free_node(ext4fs_file, &ext4fs_root->diropen);
-		ext4fs_file = NULL;
-	}
-	if (ext4fs_root != NULL) {
-		free(ext4fs_root);
-		ext4fs_root = NULL;
-	}
 	if (ext4fs_indir1_block != NULL) {
 		free(ext4fs_indir1_block);
 		ext4fs_indir1_block = NULL;
@@ -1870,6 +1874,19 @@ void ext4fs_close(void)
 		ext4fs_indir3_blkno = -1;
 	}
 }
+void ext4fs_close(void)
+{
+	if ((ext4fs_file != NULL) && (ext4fs_root != NULL)) {
+		ext4fs_free_node(ext4fs_file, &ext4fs_root->diropen);
+		ext4fs_file = NULL;
+	}
+	if (ext4fs_root != NULL) {
+		free(ext4fs_root);
+		ext4fs_root = NULL;
+	}
+
+	ext4fs_reinit_global();
+}
 
 int ext4fs_iterate_dir(struct ext2fs_node *dir, char *name,
 				struct ext2fs_node **fnode, int *ftype)
diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index 3db22f8..c42add9 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -562,6 +562,7 @@ static int ext4fs_delete_file(int inodeno)
 
 	ext4fs_update();
 	ext4fs_deinit();
+	ext4fs_reinit_global();
 
 	if (ext4fs_init() != 0) {
 		printf("error in File System init\n");
diff --git a/include/ext4fs.h b/include/ext4fs.h
index aacb147..fbbb002 100644
--- a/include/ext4fs.h
+++ b/include/ext4fs.h
@@ -133,6 +133,7 @@ int ext4fs_open(const char *filename);
 int ext4fs_read(char *buf, unsigned len);
 int ext4fs_mount(unsigned part_length);
 void ext4fs_close(void);
+void ext4fs_reinit_global(void);
 int ext4fs_ls(const char *dirname);
 int ext4fs_exists(const char *filename);
 void ext4fs_free_node(struct ext2fs_node *node, struct ext2fs_node *currroot);
-- 
1.7.10.4



More information about the U-Boot mailing list