Re: [PATCH] mm: fix mapping_seek_hole_data() overflow on last page

[Jan Kara]

On Tue 30-06-26 11:12:26, Zhen Yan wrote:
> A local unprivileged process can create a shmem/tmpfs file with
> i_size == LLONG_MAX using memfd_create() and fallocate().  If the last
> page is present in the page cache, lseek(SEEK_HOLE) on that page
> returns 0x8000000000000000 as a successful offset, which is LLONG_MIN
> when stored in loff_t.
> 
> The same file has readable data at the last byte, but SEEK_DATA from
> that offset returns ENXIO.
> 
> The overflow is in mapping_seek_hole_data():
> 
>   pos = round_up((u64)pos + 1, seek_size);
> 
> For the final page below LLONG_MAX, the next page boundary is
> 0x8000000000000000, which is then used as a signed file offset.
> 
> This violates the lseek() ABI for shmem/tmpfs: a successful seek
> returns a negative offset, and SEEK_DATA can miss readable data at EOF.
> Hole-aware copy, backup, and sparse-file scanners may skip the last
> cached byte range.
> 
> Keep mapping_seek_hole_data() inside its documented [start, end)
> search range by clamping the rounded-up position to end before passing
> it to folio_seek_hole_data().
> 
> Signed-off-by: Zhen Yan <yanzhen20011121@xxxxxxx>
> ---
>  mm/filemap.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 5af62e6abca5..22a1534ec8b3 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3234,6 +3234,8 @@ loff_t mapping_seek_hole_data(struct address_space *mapping, loff_t start,
> 
>  		seek_size = seek_folio_size(&xas, folio);
>  		pos = round_up((u64)pos + 1, seek_size);
> +		if (pos > end)
> +			pos = end;

So I agree with your analysis of the problem but I'm not sure how this is
supposed to fix it. If pos was offset of the last folio - that is
LLONG_MAX - PAGE_SIZE + 1, then round_up() ends at LLONG_MAX + 1, which
readily overflows to LLONG_MIN (pos is loff_t). So pos > end will not be
true and nothing changes. Am I missing something? So I think you rather
need the condition to check for something like pos < 0. Or maybe use
check_add_overflow() to make the overflow checking more explicit.

								Honza


>  		start = folio_seek_hole_data(&xas, mapping, folio, start, pos,
>  				seek_data);
>  		if (start < pos)
> --
> 2.25.1
> 
> Sorry, my previous email may have been lost due to network
> filtering issues.  Resending the revised patch with the
> requested changes (tabs preserved, CCs added, signoff added).
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR