Re: [PATCH v2 2/5] KVM: guest_memfd: Fix possible signed integer overflow

[Sean Christopherson <seanjc@google.com>]

On Wed, May 27, 2026, Ackerley Tng wrote:
> Sean Christopherson <seanjc@google.com> writes:
> > I like pgoff_t more than size_t because, for KVM, it's really all about addressing
> > memory, thanks to the offset into guest_memfd being associated 1:1 with a GPA.
> 
> The offset into guest_memfd is associated 1:1 with a GPA, and this
> offset is
> 
>     offset = index << PAGE_SHIFT
> 
> > It's not perfect, because GPAs are tracked as 64-bit values, whereas the kernel
> > restricts itself to "unsigned long".  But that's a non-issue in practice since
> > guest_memfd is 64-bit only.
> >
> > But conceptually, I like tracking the gmem offset as a pgoff_t to tie it back
> > to using GPAs to offset/index into gmem.  And for all intents and purposes, gmem
> > is nothing more than a glorified pagecache :-)
> 
> So we actually want to use u64s for gmem offsets (where offset = index
> << PAGE_SHIFT), 

Hrm, poking around, I guess what we really should use for the byte offset is
uoff_t.  My only hesitation to using uoff_t was that it's hardly used anywhere,
but it does seem to fix exactly what we're trying to do.

I don't want to use a raw u64, because I dislike using u{8,16,32,64} (in KVM)
unless something absolutely _must_ be that size (and ideally _exactly_ that size).
Limiting use of raw uNN helps identify fields/variables that correspond to some
hardware asset, versus fields/variables that just need to be "big enough".  It's
not a 100% comprehensive rule of anything, e.g. there are still many "naturally
sized" hardware assets that need to be tracked with "unsigned long", but I still
find it helpful/valuable to highlight hardware-derived fields/variables.

> and pgoff_t for indices, since indices (aka page
> offsets) are semantically the offset, counted in units of pages?

Yeah, I agree the distinction will help us differentiate between byte offsets
and pfn offsets, especially with another compile-time assert to show the
relationship:

	BUILD_BUG_ON(sizeof(gpa_t) != sizeof(offset));
	BUILD_BUG_ON(sizeof(gfn_t) != sizeof(slot->gmem.pgoff));

> I pulled this conclusion together from filemap-related code like
> filemap_add_folio() takes a pgoff_t index, so I thought gmem should
> follow that and stick with pgoff_t for index/indices.