linux-kernel - Re: [PATCH RESEND] filemap: Fix bounds checking in filemap

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

Message-ID: <ZzDphC-x1XEFlDvD@kernel.org>
Date: Sun, 10 Nov 2024 12:12:36 -0500
From: Mike Snitzer <snitzer@...nel.org>
To: Matthew Wilcox <willy@...radead.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christian Brauner <brauner@...nel.org>
Cc: trondmy@...nel.org, linux-fsdevel@...r.kernel.org,
	linux-nfs@...r.kernel.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org
Subject: Re: [PATCH RESEND] filemap: Fix bounds checking in filemap_read()

On Fri, Oct 25, 2024 at 08:32:57AM -0400, trondmy@...nel.org wrote:
> From: Trond Myklebust <trond.myklebust@...merspace.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@...nel.org>
> Fixes: c2a9737f45e2 ("vfs,mm: fix a dead loop in truncate_inode_pages_range()")
> Tested-by: Mike Snitzer <snitzer@...nel.org>
> Signed-off-by: Trond Myklebust <trond.myklebust@...merspace.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