Re: [PATCH RESEND] filemap: Fix bounds checking in filemap_read()

[Mike Snitzer <snitzer@kernel.org>]

On Fri, Oct 25, 2024 at 08:32:57AM -0400, trondmy@kernel.org wrote:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> 
> If the caller supplies an iocb->ki_pos value that is close to the
> filesystem upper limit, and an iterator with a count that causes us to
> overflow that limit, then filemap_read() enters an infinite loop.
> 
> This behaviour was discovered when testing xfstests generic/525 with the
> "localio" optimisation for loopback NFS mounts.
> 
> Reported-by: Mike Snitzer <snitzer@kernel.org>
> Fixes: c2a9737f45e2 ("vfs,mm: fix a dead loop in truncate_inode_pages_range()")
> Tested-by: Mike Snitzer <snitzer@kernel.org>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
>  mm/filemap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 36d22968be9a..56fa431c52af 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2625,7 +2625,7 @@ ssize_t filemap_read(struct kiocb *iocb, struct iov_iter *iter,
>  	if (unlikely(!iov_iter_count(iter)))
>  		return 0;
>  
> -	iov_iter_truncate(iter, inode->i_sb->s_maxbytes);
> +	iov_iter_truncate(iter, inode->i_sb->s_maxbytes - iocb->ki_pos);
>  	folio_batch_init(&fbatch);
>  
>  	do {
> -- 
> 2.47.0
> 
> 

Hi,

This mm fix is still needed for 6.12.  Otherwise we're exposed to an
infinite loop that is easily triggered by xfstests generic/525 when
running against 6.12's new NFS LOCALIO feature.

The irony of the original "dead loop" fix (commit c2a9737f45e2) itself
having introduced the potential for infinite loop is amusing.

Thanks,
Mike