From mboxrd@z Thu Jan  1 00:00:00 1970
From: Lukasz Majewski <l.majewski@samsung.com>
Date: Fri, 19 Aug 2016 17:17:52 +0200
Subject: [U-Boot] [PATCH 2/2] ext4: scan all directory blocks when
 looking up an entry
In-Reply-To: <6ec7a21014ed4ede81500ba5205f7e5a@rwthex-w2-b.rwth-ad.de>
References: <20160819000526.14827-1-stefan.bruens@rwth-aachen.de>
 <6ec7a21014ed4ede81500ba5205f7e5a@rwthex-w2-b.rwth-ad.de>
Message-ID: <20160819171752.4ff870c0@amdc2363>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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