lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <6e6bb868-7107-3528-db6d-0ddc275f6326@huawei.com> Date: Thu, 16 Feb 2023 15:25:23 +0800 From: zhanchengbin <zhanchengbin1@...wei.com> To: Jan Kara <jack@...e.cz> CC: <tytso@....edu>, <jack@...e.com>, <linux-ext4@...r.kernel.org>, <yi.zhang@...wei.com>, <linfeilong@...wei.com>, <liuzhiqiang26@...wei.com>, kernel test robot <lkp@...el.com> Subject: Re: [PATCH v5 2/2] ext4: clear the verified flag of the modified leaf or idx if error The last patch did not take into account path[0].p_bh == NULL, so I reworked the code. diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 0f95e857089e..05585afae0db 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -1750,13 +1750,19 @@ static int ext4_ext_correct_indexes(handle_t *handle, struct inode *inode, break; err = ext4_ext_get_access(handle, inode, path + k); if (err) - break; + goto clean; path[k].p_idx->ei_block = border; err = ext4_ext_dirty(handle, inode, path + k); if (err) - break; + goto clean; } + return 0; +clean: + while (k++ < depth) { + /* k here will not be 0, so don't consider the case where path[0].p_bh is NULL */ + clear_buffer_verified(path[k].p_bh); + } return err; } @@ -2304,6 +2310,7 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode, { int err; ext4_fsblk_t leaf; + int b_depth = depth; /* free index block */ depth--; @@ -2339,11 +2346,18 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode, path--; err = ext4_ext_get_access(handle, inode, path); if (err) - break; + goto clean; path->p_idx->ei_block = (path+1)->p_idx->ei_block; err = ext4_ext_dirty(handle, inode, path); if (err) - break; + goto clean; + } + return 0; + +clean: + while (depth++ < b_depth - 1) { + /* depth here will not be 0, so don't consider the case where path[0].p_bh is NULL */ + clear_buffer_verified(path[depth].p_bh); } return err; } On 2023/2/14 20:52, Jan Kara wrote: > > This would be more understandable as: > > if (k >= 0) > while (k++ < depth) > ... > > Also the loop is IMO wrong because it will run with k == depth as well (due > to post-increment) and that is not initialized. Furthermore it will run > also if we exit the previous loop due to: > > /* change all left-side indexes */ > if (path[k+1].p_idx != EXT_FIRST_INDEX(path[k+1].p_hdr)) > break; > > which is unwanted as well. Which suggests that you didn't test your changes > much (if at all...). So please make sure your changes are tested next time. > Thank you! > > Honza I only ran xfstest locally. Do you have any better suggestions? Thanks, - bin. > >> >> return err; >> } >> @@ -2304,6 +2306,7 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode, >> { >> int err; >> ext4_fsblk_t leaf; >> + int b_depth = depth;
Powered by blists - more mailing lists