linux-kernel - Re: [PATCH] xfs: Add error handling for xfs_reflink_cancel_cow

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] [day] [month] [year] [list]

Message-ID: <20250122032117.GS3557553@frogsfrogsfrogs>
Date: Tue, 21 Jan 2025 19:21:17 -0800
From: "Darrick J. Wong" <djwong@...nel.org>
To: Wentao Liang <vulab@...as.ac.cn>
Cc: Chandan Babu R <chandanbabu@...nel.org>,
	Allison Henderson <allison.henderson@...cle.com>,
	Dave Chinner <dchinner@...hat.com>,
	Catherine Hoang <catherine.hoang@...cle.com>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] xfs: Add error handling for xfs_reflink_cancel_cow_range

On Mon, Jan 20, 2025 at 11:46:24PM +0800, Wentao Liang wrote:
> In xfs_inactive(), xfs_reflink_cancel_cow_range() is called
> without error handling, risking unnoticed failures and
> inconsistent behavior compared to other parts of the code.
> 
> Fix this issue by adding an error handling for the
> xfs_reflink_cancel_cow_range(), improving code robustness.
> 
> Fixes: 6231848c3aa5 ("xfs: check for cow blocks before trying to clear them")
> Signed-off-by: Wentao Liang <vulab@...as.ac.cn>
> ---
>  fs/xfs/xfs_inode.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c8ad2606f928..1ff514b6c035 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1404,8 +1404,11 @@ xfs_inactive(
>  		goto out;
>  
>  	/* Try to clean out the cow blocks if there are any. */
> -	if (xfs_inode_has_cow_data(ip))
> -		xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, true);
> +	if (xfs_inode_has_cow_data(ip)) {
> +		error = xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, true);
> +		if (error)
> +			goto out;

If memory serves, we ignored the error return here on the grounds that
the worst that can happen is that we leak some cow staging reservations,
so we should continue freeing the file in the ondisk metadata.

That said, I /think/ the error case corresponds to the log being shut
down or going down due to a corruption error during the cow blocks
cancellation.  So I guess this can go ahead with:

Cc: <stable@...r.kernel.org> # v4.17
Reviewed-by: "Darrick J. Wong" <djwong@...nel.org>

--D

> +	}
>  
>  	if (VFS_I(ip)->i_nlink != 0) {
>  		/*
> -- 
> 2.42.0.windows.2
>