[PATCH] filemap: avoid truncating 64-bit offset to 32 bits

[PATCH] filemap: avoid truncating 64-bit offset to 32 bits

[Marco Nelissen]

on 32-bit kernels, folio_seek_hole_data() was inadvertently truncating a
64-bit value to 32 bits, leading to a possible infinite loop when writing
to an xfs filesystem.

Signed-off-by: Marco Nelissen <marco.nelissen@gmail.com>
---
 mm/filemap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index f61cf51c2238..f5c93683dbcf 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3005,7 +3005,7 @@ static inline loff_t folio_seek_hole_data(struct xa_state *xas,
 		if (ops->is_partially_uptodate(folio, offset, bsz) ==
 							seek_data)
 			break;
-		start = (start + bsz) & ~(bsz - 1);
+		start = (start + bsz) & ~((u64)bsz - 1);
 		offset += bsz;
 	} while (offset < folio_size(folio));
 unlock:
-- 
2.39.5

Re: [PATCH] filemap: avoid truncating 64-bit offset to 32 bits

[Andrew Morton]

On Thu,  2 Jan 2025 11:04:11 -0800 Marco Nelissen <marco.nelissen@gmail.com> wrote:

> on 32-bit kernels, folio_seek_hole_data() was inadvertently truncating a
> 64-bit value to 32 bits, leading to a possible infinite loop when writing
> to an xfs filesystem.
> 
> ...
>
> +++ b/mm/filemap.c
> @@ -3005,7 +3005,7 @@ static inline loff_t folio_seek_hole_data(struct xa_state *xas,
>  		if (ops->is_partially_uptodate(folio, offset, bsz) ==
>  							seek_data)
>  			break;
> -		start = (start + bsz) & ~(bsz - 1);
> +		start = (start + bsz) & ~((u64)bsz - 1);
>  		offset += bsz;
>  	} while (offset < folio_size(folio));
>  unlock:

Thanks.  I'll add

Fixes: 54fa39ac2e00b ("iomap: use mapping_seek_hole_data")
Cc: <stable@vger.kernel.org>

The

	offset = offset_in_folio(folio, start) & ~(bsz - 1);

a few lines earlier is worrisome.  I wonder if we should simply make
`bsz' and `offset' have type u64 and sleep well at night.

Re: [PATCH] filemap: avoid truncating 64-bit offset to 32 bits

[Darrick J. Wong]

On Thu, Jan 02, 2025 at 02:10:15PM -0800, Andrew Morton wrote:
> On Thu,  2 Jan 2025 11:04:11 -0800 Marco Nelissen <marco.nelissen@gmail.com> wrote:
> 
> > on 32-bit kernels, folio_seek_hole_data() was inadvertently truncating a
> > 64-bit value to 32 bits, leading to a possible infinite loop when writing
> > to an xfs filesystem.
> > 
> > ...
> >
> > +++ b/mm/filemap.c
> > @@ -3005,7 +3005,7 @@ static inline loff_t folio_seek_hole_data(struct xa_state *xas,
> >  		if (ops->is_partially_uptodate(folio, offset, bsz) ==
> >  							seek_data)
> >  			break;
> > -		start = (start + bsz) & ~(bsz - 1);
> > +		start = (start + bsz) & ~((u64)bsz - 1);
> >  		offset += bsz;
> >  	} while (offset < folio_size(folio));
> >  unlock:
> 
> Thanks.  I'll add
> 
> Fixes: 54fa39ac2e00b ("iomap: use mapping_seek_hole_data")
> Cc: <stable@vger.kernel.org>
> 
> The
> 
> 	offset = offset_in_folio(folio, start) & ~(bsz - 1);
> 
> a few lines earlier is worrisome.  I wonder if we should simply make
> `bsz' and `offset' have type u64 and sleep well at night.

I think that callsite is ok because offset_in_folio() should never
return a value larger than 2^31 ... right?  IOWs, only the second case
needs casting because @start is loff_t.

--D