From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Majewski Date: Fri, 14 Oct 2016 12:29:54 +0200 Subject: [U-Boot] [PATCH 2/3] ext4: Fix handling of direntlen in unlink_filename In-Reply-To: References: <20161009181528.26022-1-stefan.bruens@rwth-aachen.de> Message-ID: <20161014122954.5d6be6e9@amdc2363> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Stefan, > The direntlen checks were quite bogus, i.e. the loop termination used > "len + offset == blocksize" (exact match only), and checked for a > direntlen less than 0. The latter can never happen as the len is > unsigned, this has been reported by Coverity, CID 153384. > > Use the same code as in search_dir for directory traversal. This code > has the correct checks for direntlen >= sizeof(struct dirent), and > offset < blocksize. > > Signed-off-by: Stefan Br?ns > --- > fs/ext4/ext4_common.c | 45 > +++++++++++++++++---------------------------- 1 file changed, 17 > insertions(+), 28 deletions(-) > > diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c > index 699a640..11be0b7 100644 > --- a/fs/ext4/ext4_common.c > +++ b/fs/ext4/ext4_common.c > @@ -854,16 +854,15 @@ fail: > > static int unlink_filename(char *filename, unsigned int blknr) > { > - int templength = 0; > - int status, inodeno; > - int found = 0; > + int status; > + int inodeno = 0; > int offset; > char *block_buffer = NULL; > struct ext2_dirent *dir = NULL; > - struct ext2_dirent *previous_dir = NULL; > - char *ptr = NULL; > + struct ext2_dirent *previous_dir; > struct ext_filesystem *fs = get_fs(); > int ret = -1; > + char *direntname; > > block_buffer = zalloc(fs->blksz); > if (!block_buffer) > @@ -877,20 +876,18 @@ static int unlink_filename(char *filename, > unsigned int blknr) > if (ext4fs_log_journal(block_buffer, blknr)) > goto fail; > - dir = (struct ext2_dirent *)block_buffer; > - ptr = (char *)dir; > offset = 0; > - while (le16_to_cpu(dir->direntlen) >= 0) { > - /* > - * blocksize-offset 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 > - */ > + do { > + previous_dir = dir; > + dir = (struct ext2_dirent *)(block_buffer + offset); > + direntname = (char *)(dir) + sizeof(struct > ext2_dirent); + > + int direntlen = le16_to_cpu(dir->direntlen); > + if (direntlen < sizeof(struct ext2_dirent)) > + break; > + > if (dir->inode && (strlen(filename) == dir->namelen) > && > - (strncmp(ptr + sizeof(struct ext2_dirent), > - filename, dir->namelen) == 0)) { > - printf("file found, deleting\n"); > + (strncmp(direntname, filename, dir->namelen) == > 0)) { inodeno = le32_to_cpu(dir->inode); > if (previous_dir) { > uint16_t new_len; > @@ -900,23 +897,15 @@ static int unlink_filename(char *filename, > unsigned int blknr) } else { > dir->inode = 0; > } > - found = 1; > break; > } > > - if (fs->blksz - offset == > le16_to_cpu(dir->direntlen)) > - break; > + offset += direntlen; > > - /* traversing the each directory entry */ > - templength = le16_to_cpu(dir->direntlen); > - offset = offset + templength; > - previous_dir = dir; > - dir = (struct ext2_dirent *)((char *)dir + > templength); > - ptr = (char *)dir; > - } > + } while (offset < fs->blksz); > > + if (inodeno > 0) { > > - if (found == 1) { > if (ext4fs_put_metadata(block_buffer, blknr)) > goto fail; > ret = inodeno; Reviewed-by: Lukasz Majewski -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group