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
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220901083706.mjewb45dufzxcfdk@quack3>
Date:   Thu, 1 Sep 2022 10:37:06 +0200
From:   Jan Kara <jack@...e.cz>
To:     Ye Bin <yebin10@...wei.com>
Cc:     tytso@....edu, adilger.kernel@...ger.ca,
        linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
        jack@...e.cz
Subject: Re: [PATCH -next] ext4: ensure data forced to disk when rename

On Thu 01-09-22 14:26:57, Ye Bin wrote:
> There is issue that data lost when rename new file. Reason is dioread_nolock is
> enabled default now, which map blocks will mark extent unwritten. But inode
> size is changed after submit io. Then read file will return zero if extent is
> unwritten.
> To solve above isuue, wait all data force to disk before rename when enable
> dioread_nolock and enable delay allocate.
> 
> Signed-off-by: Ye Bin <yebin10@...wei.com>

Well, but it was always like that. We do not guarantee that the data is
securely on disk before rename - userspace is responsible for that if it
needs this guarantee. We just want to make the time window shorter and so
start data writeback because lot of userspace is careless. But waiting for
data writeback before rename will significantly slow down some workloads
and IMHO without a good reason.

								Honza

> ---
>  fs/ext4/ext4.h  |  1 +
>  fs/ext4/inode.c | 16 ++++++++++++++++
>  fs/ext4/namei.c |  2 +-
>  3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 9bca5565547b..111296259b89 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2998,6 +2998,7 @@ extern int ext4_break_layouts(struct inode *);
>  extern int ext4_punch_hole(struct file *file, loff_t offset, loff_t length);
>  extern void ext4_set_inode_flags(struct inode *, bool init);
>  extern int ext4_alloc_da_blocks(struct inode *inode);
> +extern void ext4_flush_data(struct inode *inode);
>  extern void ext4_set_aops(struct inode *inode);
>  extern int ext4_writepage_trans_blocks(struct inode *);
>  extern int ext4_chunk_trans_blocks(struct inode *, int nrblocks);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 601214453c3a..4df7ffd3f1b0 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3133,6 +3133,22 @@ int ext4_alloc_da_blocks(struct inode *inode)
>  	return filemap_flush(inode->i_mapping);
>  }
>  
> +void ext4_flush_data(struct inode *inode)
> +{
> +	if (!EXT4_I(inode)->i_reserved_data_blocks)
> +		return;
> +
> +	if (filemap_flush(inode->i_mapping))
> +		return;
> +
> +	if (test_opt(inode->i_sb, DELALLOC) &&
> +	    ext4_should_dioread_nolock(inode) &&
> +	    atomic_read(&EXT4_I(inode)->i_unwritten))
> +		filemap_fdatawait(inode->i_mapping);
> +
> +	return;
> +}
> +
>  /*
>   * bmap() is special.  It gets used by applications such as lilo and by
>   * the swapper to find the on-disk block of a specific piece of data.
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 3a31b662f661..030402fe3b9f 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -3820,7 +3820,7 @@ static int ext4_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
>  		}
>  	}
>  	if (new.inode && !test_opt(new.dir->i_sb, NO_AUTO_DA_ALLOC))
> -		ext4_alloc_da_blocks(old.inode);
> +		ext4_flush_data(old.inode);
>  
>  	credits = (2 * EXT4_DATA_TRANS_BLOCKS(old.dir->i_sb) +
>  		   EXT4_INDEX_EXTRA_TRANS_BLOCKS + 2);
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ