* [U-Boot] [PATCH 1/2] ext4: Avoid corruption of directories with hash tree indexes [not found] <20160819000526.14827-1-stefan.bruens@rwth-aachen.de> @ 2016-08-19 0:05 ` Stefan Brüns 2016-08-19 15:11 ` Lukasz Majewski 2016-08-19 0:05 ` [U-Boot] [PATCH 2/2] ext4: scan all directory blocks when looking up an entry Stefan Brüns 1 sibling, 1 reply; 4+ messages in thread From: Stefan Brüns @ 2016-08-19 0:05 UTC (permalink / raw) To: u-boot 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> --- 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 4235b95..ccc8a9c 100644 --- a/fs/ext4/ext4_write.c +++ b/fs/ext4/ext4_write.c @@ -864,6 +864,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.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH 1/2] ext4: Avoid corruption of directories with hash tree indexes 2016-08-19 0:05 ` [U-Boot] [PATCH 1/2] ext4: Avoid corruption of directories with hash tree indexes Stefan Brüns @ 2016-08-19 15:11 ` Lukasz Majewski 0 siblings, 0 replies; 4+ messages in thread From: Lukasz Majewski @ 2016-08-19 15:11 UTC (permalink / raw) To: u-boot Hi Stefan, > 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> > --- > 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 4235b95..ccc8a9c 100644 > --- a/fs/ext4/ext4_write.c > +++ b/fs/ext4/ext4_write.c > @@ -864,6 +864,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 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] 4+ messages in thread
* [U-Boot] [PATCH 2/2] ext4: scan all directory blocks when looking up an entry [not found] <20160819000526.14827-1-stefan.bruens@rwth-aachen.de> 2016-08-19 0:05 ` [U-Boot] [PATCH 1/2] ext4: Avoid corruption of directories with hash tree indexes Stefan Brüns @ 2016-08-19 0:05 ` Stefan Brüns 2016-08-19 15:17 ` Lukasz Majewski 1 sibling, 1 reply; 4+ messages in thread From: Stefan Brüns @ 2016-08-19 0:05 UTC (permalink / raw) To: u-boot 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> --- fs/ext4/ext4_common.c | 74 +++++++++++++++++++++++---------------------------- 1 file changed, 33 insertions(+), 41 deletions(-) diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index 8d0013e..aa72b0d 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -500,64 +500,53 @@ 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; struct ext2_dirent *dir = NULL; struct ext_filesystem *fs = get_fs(); + uint32_t directory_blocks; - /* 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(g_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; + offset = 0; 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; - } + while (le16_to_cpu(dir->direntlen) >= 8) { + if (dir->inode && (strlen(dirname) == dir->namelen) && + (strncmp(dirname, &dir[1], dir->namelen) == 0)) { + inodeno = le32_to_cpu(dir->inode); + break; } - if (fs->blksz - totalbytes == le16_to_cpu(dir->direntlen)) + offset += le16_to_cpu(dir->direntlen); + if (offset >= fs->blksz) 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; + dir = (struct ext2_dirent *)(block_buffer + offset); } - free(block_buffer); - block_buffer = NULL; - - if (inodeno > 0) + if (inodeno > 0) { + free(block_buffer); return inodeno; + } } fail: @@ -809,14 +798,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.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH 2/2] ext4: scan all directory blocks when looking up an entry 2016-08-19 0:05 ` [U-Boot] [PATCH 2/2] ext4: scan all directory blocks when looking up an entry Stefan Brüns @ 2016-08-19 15:17 ` Lukasz Majewski 0 siblings, 0 replies; 4+ messages in thread From: Lukasz Majewski @ 2016-08-19 15:17 UTC (permalink / raw) To: u-boot Hi Stefan, > 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> > --- > fs/ext4/ext4_common.c | 74 > +++++++++++++++++++++++---------------------------- 1 file changed, > 33 insertions(+), 41 deletions(-) > > diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c > index 8d0013e..aa72b0d 100644 > --- a/fs/ext4/ext4_common.c > +++ b/fs/ext4/ext4_common.c > @@ -500,64 +500,53 @@ 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; > struct ext2_dirent *dir = NULL; > struct ext_filesystem *fs = get_fs(); > + uint32_t directory_blocks; > > - /* 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(g_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; > > + offset = 0; > 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; > - } > + while (le16_to_cpu(dir->direntlen) >= 8) { > + if (dir->inode && (strlen(dirname) == > dir->namelen) && > + (strncmp(dirname, &dir[1], dir->namelen) > == 0)) { > + inodeno = le32_to_cpu(dir->inode); > + break; > } > > - if (fs->blksz - totalbytes == > le16_to_cpu(dir->direntlen)) > + offset += le16_to_cpu(dir->direntlen); > + if (offset >= fs->blksz) > 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; > + dir = (struct ext2_dirent *)(block_buffer + > offset); } > > - free(block_buffer); > - block_buffer = NULL; > - > - if (inodeno > 0) > + if (inodeno > 0) { > + free(block_buffer); > return inodeno; > + } > } > > fail: > @@ -809,14 +798,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); 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] 4+ messages in thread
end of thread, other threads:[~2016-08-19 15:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20160819000526.14827-1-stefan.bruens@rwth-aachen.de>
2016-08-19 0:05 ` [U-Boot] [PATCH 1/2] ext4: Avoid corruption of directories with hash tree indexes Stefan Brüns
2016-08-19 15:11 ` Lukasz Majewski
2016-08-19 0:05 ` [U-Boot] [PATCH 2/2] ext4: scan all directory blocks when looking up an entry Stefan Brüns
2016-08-19 15:17 ` Lukasz Majewski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox