On Tue 05-01-21 14:28:57, yangerkun wrote:
> We got a "deleted inode referenced" warning cross our fsstress test. The
> bug can be reproduced easily with following steps:
>
> cd /dev/shm
> mkdir test/
> fallocate -l 128M img
> mkfs.ext4 -b 1024 img
> mount img test/
> dd if=/dev/zero of=test/foo bs=1M count=128
> mkdir test/dir/ && cd test/dir/
> for ((i=0;i<1000;i++)); do touch file$i; done # consume all block
> cd ~ && renameat2(AT_FDCWD, /dev/shm/test/dir/file1, AT_FDCWD,
> /dev/shm/test/dir/dst_file, RENAME_WHITEOUT) # ext4_add_entry in
> ext4_rename will return ENOSPC!!
> cd /dev/shm/ && umount test/ && mount img test/ && ls -li test/dir/file1
> We will get the output:
> "ls: cannot access 'test/dir/file1': Structure needs cleaning"
> and the dmesg show:
> "EXT4-fs error (device loop0): ext4_lookup:1626: inode #2049: comm ls:
> deleted inode referenced: 139"
>
> ext4_rename will create a special inode for whiteout and use this 'ino'
> to replace the source file's dir entry 'ino'. Once error happens
> latter(the error above was the ENOSPC return from ext4_add_entry in
> ext4_rename since all space has been consumed), the cleanup do drop the
> nlink for whiteout, but forget to restore 'ino' with source file. This
> will trigger the bug describle as above.
>
> Signed-off-by: yangerkun <yangerkun@xxxxxxxxxx>
Thanks! The patch looks good to me now. You can add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/ext4/namei.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index b17a082b7db1..90f7ebeb69c8 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -3593,9 +3593,6 @@ static int ext4_setent(handle_t *handle, struct ext4_renament *ent,
> return retval2;
> }
> }
> - brelse(ent->bh);
> - ent->bh = NULL;
> -
> return retval;
> }
>
> @@ -3794,6 +3791,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> }
> }
>
> + old_file_type = old.de->file_type;
> if (IS_DIRSYNC(old.dir) || IS_DIRSYNC(new.dir))
> ext4_handle_sync(handle);
>
> @@ -3821,7 +3819,6 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> force_reread = (new.dir->i_ino == old.dir->i_ino &&
> ext4_test_inode_flag(new.dir, EXT4_INODE_INLINE_DATA));
>
> - old_file_type = old.de->file_type;
> if (whiteout) {
> /*
> * Do this before adding a new entry, so the old entry is sure
> @@ -3919,15 +3916,19 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> retval = 0;
>
> end_rename:
> - brelse(old.dir_bh);
> - brelse(old.bh);
> - brelse(new.bh);
> if (whiteout) {
> - if (retval)
> + if (retval) {
> + ext4_setent(handle, &old,
> + old.inode->i_ino, old_file_type);
> drop_nlink(whiteout);
> + }
> unlock_new_inode(whiteout);
> iput(whiteout);
> +
> }
> + brelse(old.dir_bh);
> + brelse(old.bh);
> + brelse(new.bh);
> if (handle)
> ext4_journal_stop(handle);
> return retval;
> --
> 2.25.4
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
