Re: [PATCH] udf: Fix data loss when converting inline inodes to out of line

On Wed 29-07-26 16:45:48, Jan Kara wrote:
> When udf_expand_file_adinicb() converts file from inline format to out
> of line, we use filemap_fdatawrite() to writeout the data to the new
> blocks. However since 36580ed08776 ("udf: Do not allocate blocks on page
> writeback") the writeback actually doesn't allocate the new block and
> the folio dirty bit is just silently cleared. Thus unless the file is
> written to after the conversion (as it can easily happen in case of
> truncate up), the data is just lost. Fix the problem by explicitely
> allocating the block underlying the data before starting writeback.
> 
> Fixes: 36580ed08776 ("udf: Do not allocate blocks on page writeback")
> CC: [email protected]
> Signed-off-by: Jan Kara <[email protected]>

Sorry, this was an old version of the patch which missed prerequisite
moving of udf_map_block() declarations up in the file.

								Honza

> ---
>  fs/udf/inode.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> I plan to merge this fix through my tree.
> 
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index b8b9b7132115..b5e374f355b2 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -405,6 +405,10 @@ int udf_expand_file_adinicb(struct inode *inode)
>  {
>  	struct folio *folio;
>  	struct udf_inode_info *iinfo = UDF_I(inode);
> +	struct udf_map_rq map = {
> +		.lblk = 0,
> +		.iflags = UDF_MAP_CREATE,
> +	};
>  	int err;
>  
>  	WARN_ON_ONCE(!inode_is_locked(inode));
> @@ -434,20 +438,27 @@ int udf_expand_file_adinicb(struct inode *inode)
>  		iinfo->i_alloc_type = ICBTAG_FLAG_AD_SHORT;
>  	else
>  		iinfo->i_alloc_type = ICBTAG_FLAG_AD_LONG;
> +	up_write(&iinfo->i_data_sem);
> +
> +	/* Allocate the block underlying the data */
> +	err = udf_map_block(inode, &map);
> +	if (err < 0)
> +		goto restore;
> +
>  	folio_mark_dirty(folio);
>  	folio_unlock(folio);
> -	up_write(&iinfo->i_data_sem);
>  	err = filemap_fdatawrite(inode->i_mapping);
>  	if (err) {
>  		/* Restore everything back so that we don't lose data... */
>  		folio_lock(folio);
> +restore:
>  		down_write(&iinfo->i_data_sem);
>  		memcpy_from_folio(iinfo->i_data + iinfo->i_lenEAttr,
>  				folio, 0, inode->i_size);
> -		folio_unlock(folio);
>  		iinfo->i_alloc_type = ICBTAG_FLAG_AD_IN_ICB;
>  		iinfo->i_lenAlloc = inode->i_size;
>  		up_write(&iinfo->i_data_sem);
> +		folio_unlock(folio);
>  	}
>  	folio_put(folio);
>  	mark_inode_dirty(inode);
> -- 
> 2.51.0
> 
-- 
Jan Kara <[email protected]>
SUSE Labs, CR