linux-kernel - Re: [PATCH] xfs: add missing cmap->br_state = XFS_EXT

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220218054032.GO8313@magnolia>
Date:   Thu, 17 Feb 2022 21:40:32 -0800
From:   "Darrick J. Wong" <djwong@...nel.org>
To:     Gao Xiang <hsiangkao@...ux.alibaba.com>
Cc:     xfs <linux-xfs@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] xfs: add missing cmap->br_state = XFS_EXT_NORM update

On Thu, Feb 17, 2022 at 05:55:42PM +0800, Gao Xiang wrote:
> COW extents are already converted into written real extents after
> xfs_reflink_convert_cow_locked(), therefore cmap->br_state should
> reflect it.
> 
> Otherwise, there is another necessary unwritten convertion
> triggered in xfs_dio_write_end_io() for direct I/O cases.
> 
> Signed-off-by: Gao Xiang <hsiangkao@...ux.alibaba.com>

I /think/ this looks ok.  Does it test ok too?  AFAICT nothing in the
iomap/writeback machinery cares the incorrect state because we always
set IOMAP_F_SHARED (which triggers COW) so I think this is simply a fix
for directio, like you said?

--D

> ---
> From the logic itself and runtime tracing, IMO, it seems true.
> Kindly correct me here if my understanding is wrong.
> 
>  fs/xfs/xfs_reflink.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 75bd2e03cd5b..5f0a364739a5 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -424,7 +424,10 @@ xfs_reflink_allocate_cow(
>  	if (!convert_now || cmap->br_state == XFS_EXT_NORM)
>  		return 0;
>  	trace_xfs_reflink_convert_cow(ip, cmap);
> -	return xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb);
> +	error = xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb);
> +	if (!error)
> +		cmap->br_state = XFS_EXT_NORM;
> +	return error;
>  
>  out_trans_cancel:
>  	xfs_trans_cancel(tp);
> -- 
> 2.24.4
>