| Message ID | 20250102190540.1356838-1-marco.nelissen@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | filemap: avoid truncating 64-bit offset to 32 bits | expand |
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.
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
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:
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(-)