On Thu 04-04-19 17:29:52, zhangyi (F) wrote:
> Currently we capture a NULL data exposure problem after a crash or
> poweroff when append writing a file in the data=ordered mode. The
> problem is that we were not add inode to the transaction's order data
> list when updating i_disksize without new block allocation no matter
> the delay allocated block feature is enabled or not.
>
> write jbd2 writeback
> append write in allocated block
> mark buffer dirty
> update i_disksize
> mark inode dirty
> commit transaction
> write inode
> (data exposure after a crash)
> write dirty buffer
>
> It's fine in the case of new block allocation because we do this job in
> ext4_map_blocks(). To fix this problem, this patch add inode to current
> transaction's order data list after new data is copied and needing
> update i_disksize in the case of no block allocation.
>
> Fixes: 06bd3c36a733ac ("ext4: fix data exposure after a crash")
> Fixes: f3b59291a69d0b ("ext4: remove calls to ext4_jbd2_file_inode() from delalloc write path")
> Signed-off-by: zhangyi (F) <yi.zhang@xxxxxxxxxx>
Thanks for the patch. The current behavior is a deliberate decision.
data=ordered mode does guarantee there is no stale data visible in case of
crash. However it does not guarantee you cannot see zeros where data was
written.
Honza
> ---
> fs/ext4/inode.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index b32a57b..5cfa066 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1419,6 +1419,16 @@ static int ext4_write_end(struct file *file,
> if (i_size_changed || inline_data)
> ext4_mark_inode_dirty(handle, inode);
>
> + /*
> + * Updating i_disksize when extending file without block
> + * allocation, the newly written data where should be visible
> + * after transaction commit must be on transaction's ordered
> + * data list.
> + */
> + if (copied && (i_size_changed & 0x2) &&
> + ext4_should_order_data(inode))
> + ext4_jbd2_inode_add_write(handle, inode);
> +
> if (pos + len > inode->i_size && ext4_can_truncate(inode))
> /* if we have allocated more blocks and copied
> * less. We will have blocks allocated outside
> @@ -3185,6 +3195,15 @@ static int ext4_da_write_end(struct file *file,
> * bu greater than i_disksize.(hint delalloc)
> */
> ext4_mark_inode_dirty(handle, inode);
> +
> + /*
> + * Updating i_disksize when extending file without
> + * block allocation, the newly written data where
> + * should be visible after transaction commit must
> + * be on transaction's ordered data list.
> + */
> + if (ext4_should_order_data(inode))
> + ext4_jbd2_inode_add_write(handle, inode);
> }
> }
>
> --
> 2.7.4
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
