On Tuesday 27 Feb 2024 at 15:59:37 (+0100), David Hildenbrand wrote: > > > > Ah, this was something I hadn't thought about. I think both Fuad and I > > need to update our series to check the refcount rather than mapcount > > (kvm_is_gmem_mapped for Fuad, gunyah_folio_lend_safe for me). > > An alternative might be !folio_mapped() && !folio_maybe_dma_pinned(). But > checking for any unexpected references might be better (there are still some > GUP users that don't use FOLL_PIN). As a non-mm person I'm not sure to understand to consequences of holding a GUP pin to a page that is not covered by any VMA. The absence of VMAs imply that userspace cannot access the page right? Presumably the kernel can't be coerced into accessing that page either? Is that correct? > At least concurrent migration/swapout (that temporarily unmaps a folio and > can give you folio_mapped() "false negatives", which both take a temporary > folio reference and hold the page lock) should not be a concern because > guest_memfd doesn't support that yet. > > > > > > > > > Now, regarding the original question (disallow mapping the page), I see the > > > following approaches: > > > > > > 1) SIGBUS during page fault. There are other cases that can trigger > > > SIGBUS during page faults: hugetlb when we are out of free hugetlb > > > pages, userfaultfd with UFFD_FEATURE_SIGBUS. > > > > > > -> Simple and should get the job done. > > > > > > 2) folio_mmapped() + preventing new mmaps covering that folio > > > > > > -> More complicated, requires an rmap walk on every conversion. > > > > > > 3) Disallow any mmaps of the file while any page is private > > > > > > -> Likely not what you want. > > > > > > > > > Why was 1) abandoned? I looks a lot easier and harder to mess up. Why are > > > you trying to avoid page faults? What's the use case? > > > > > > > We were chatting whether we could do better than the SIGBUS approach. > > SIGBUS/FAULT usually crashes userspace, so I was brainstorming ways to > > return errors early. One difference between hugetlb and this usecase is > > that running out of free hugetlb pages isn't something we could detect > > With hugetlb reservation one can try detecting it at mmap() time. But as > reservations are not NUMA aware, it's not reliable. > > > at mmap time. In guest_memfd usecase, we should be able to detect when > > SIGBUS becomes possible due to memory being lent to guest. > > > > I can't think of a reason why userspace would want/be able to resume > > operation after trying to access a page that it shouldn't be allowed, so > > SIGBUS is functional. The advantage of trying to avoid SIGBUS was > > better/easier reporting to userspace. > > To me, it sounds conceptually easier and less error-prone to > > 1) Converting a page to private only if there are no unexpected > references (no mappings, GUP pins, ...) > 2) Disallowing mapping private pages and failing the page fault. > 3) Handling that small race window only (page lock?) > > Instead of > > 1) Converting a page to private only if there are no unexpected > references (no mappings, GUP pins, ...) and no VMAs covering it where > we could fault it in later > 2) Disallowing mmap when the range would contain any private page > 3) Handling races between mmap and page conversion The one thing that makes the second option cleaner from a userspace perspective (IMO) is that the conversion to private is happening lazily during guest faults. So whether or not an mmapped page can indeed be accessed from userspace will be entirely undeterministic as it depends on the guest faulting pattern which userspace is entirely unaware of. Elliot's suggestion would prevent spurious crashes caused by that somewhat odd behaviour, though arguably sane userspace software shouldn't be doing that to start with. To add a layer of paint to the shed, the usage of SIGBUS for something that is really a permission access problem doesn't feel appropriate. Allocating memory via guestmem and donating that to a protected guest is a way for userspace to voluntarily relinquish access permissions to the memory it allocated. So a userspace process violating that could, IMO, reasonably expect a SEGV instead of SIGBUS. By the point that signal would be sent, the page would have been accounted against that userspace process, so not sure the paging examples that were discussed earlier are exactly comparable. To illustrate that differently, given that pKVM and Gunyah use MMU-based protection, there is nothing architecturally that prevents a guest from sharing a page back with Linux as RO. Note that we don't currently support this, so I don't want to conflate this use case, but that hopefully makes it a little more obvious that this is a "there is a page, but you don't currently have the permission to access it" problem rather than "sorry but we ran out of pages" problem. Thanks, Quentin
