Re: [PATCH 2/2] iomap: constrain the file range passed to iomap_file_unshare

[Brian Foster <bfoster@redhat.com>]

On Wed, Oct 02, 2024 at 08:02:13AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> File contents can only be shared (i.e. reflinked) below EOF, so it makes
> no sense to try to unshare ranges beyond EOF.  Constrain the file range
> parameters here so that we don't have to do that in the callers.
> 
> Fixes: 5f4e5752a8a3 ("fs: add iomap_file_dirty")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  fs/dax.c               |    6 +++++-
>  fs/iomap/buffered-io.c |    6 +++++-
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/dax.c b/fs/dax.c
> index becb4a6920c6a..c62acd2812f8d 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -1305,11 +1305,15 @@ int dax_file_unshare(struct inode *inode, loff_t pos, loff_t len,
>  	struct iomap_iter iter = {
>  		.inode		= inode,
>  		.pos		= pos,
> -		.len		= len,
>  		.flags		= IOMAP_WRITE | IOMAP_UNSHARE | IOMAP_DAX,
>  	};
> +	loff_t size = i_size_read(inode);
>  	int ret;
>  
> +	if (pos < 0 || pos >= size)
> +		return 0;
> +
> +	iter.len = min(len, size - pos);
>  	while ((ret = iomap_iter(&iter, ops)) > 0)
>  		iter.processed = dax_unshare_iter(&iter);
>  	return ret;
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index c1c559e0cc07c..78ebd265f4259 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1375,11 +1375,15 @@ iomap_file_unshare(struct inode *inode, loff_t pos, loff_t len,
>  	struct iomap_iter iter = {
>  		.inode		= inode,
>  		.pos		= pos,
> -		.len		= len,
>  		.flags		= IOMAP_WRITE | IOMAP_UNSHARE,
>  	};
> +	loff_t size = i_size_read(inode);
>  	int ret;
>  
> +	if (pos < 0 || pos >= size)
> +		return 0;
> +
> +	iter.len = min(len, size - pos);
>  	while ((ret = iomap_iter(&iter, ops)) > 0)
>  		iter.processed = iomap_unshare_iter(&iter);
>  	return ret;
> 

Heh. This was pretty much my local fix when I was testing fsx unshare
range, so LGTM. Apologies, I probably should have just sent it out. It
just seemed like Julian was 90% there, but then review went off the
rails and I guess I lost interest. Anyways, thanks for the fix:

Reviewed-by: Brian Foster <bfoster@redhat.com>