diff mbox series

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

Message ID f875d790e335792eca5b925d0c2c559c4e7fa299.1729859474.git.trond.myklebust@hammerspace.com (mailing list archive)
State New
Headers show
Series [RESEND] filemap: Fix bounds checking in filemap_read() | expand

Commit Message

Trond Myklebust Oct. 25, 2024, 12:32 p.m. UTC
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(-)

Comments

Mike Snitzer Nov. 10, 2024, 5:12 p.m. UTC | #1
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
Linus Torvalds Nov. 10, 2024, 10:08 p.m. UTC | #2
On Sun, 10 Nov 2024 at 09:12, Mike Snitzer <snitzer@kernel.org> wrote:
>
> This mm fix is still needed for 6.12.

Thanks for pointing it out. I've applied it directly,

            Linus
diff mbox series

Patch

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 {