From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
Alex Williamson <alex@shazbot.org>,
bcm-kernel-feedback-list@broadcom.com,
Boris Brezillon <boris.brezillon@collabora.com>,
Christian Koenig <christian.koenig@amd.com>,
David Hildenbrand <david@kernel.org>,
dri-devel@lists.freedesktop.org, Fei Li <fei1.li@intel.com>,
Huang Rui <ray.huang@amd.com>,
linux-mm@kvack.org, linux-s390@vger.kernel.org,
Michal Hocko <mhocko@suse.com>, Peter Xu <peterx@redhat.com>,
Sergio Lopez <slp@redhat.com>,
Thomas Zimmermann <tzimmermann@suse.de>
Subject: Re: [PATCH v2 0/6] mm, drm: fix interaction of .pfn_mkwrite() with fixup_user_fault()
Date: Mon, 26 Oct 2026 06:49:56 -0700 [thread overview]
Message-ID: <at9ahKiYsVtH8kpU@google.com> (raw)
In-Reply-To: <20260804120529.1730187-1-pbonzini@redhat.com>
On Tue, Aug 04, 2026, Paolo Bonzini wrote:
> This "v2" combines three series that I have previously posted separately to gather
> reviews and tests:
>
> - kvm: apply VM_READ/VM_WRITE checks to all VMA types
> https://lore.kernel.org/kvm/20260731175834.1121005-1-pbonzini@redhat.com/
>
> - mm: pull writability check to follow_pfnmap_start()
> https://lore.kernel.org/kvm/20260731160514.1101989-1-pbonzini@redhat.com/T/#u
>
> - mm, drm: ensure .fault() does not have to be followed by .pfn_mkwrite() for write faults
> https://lore.kernel.org/kvm/20260731164341.1109827-1-pbonzini@redhat.com/
>
> All three, together, make it possible to write drivers that use .fault()
> and .pfn_mkwrite() callbacks for VM_IO|VM_PFNMAP regions, and that also
> interact correctly with users of fixup_user_fault().
>
> The problem is that if you define .pfn_mkwrite(), vma_set_page_prot()
> clears the writable PTE bit in vma->vm_page_prot, at which point
> the .fault() callback has no way to create a writable PTE. Users of
> fixup_user_fault() will then see a read-only PTE and have no clue that
> the page needs a *second* fault to reach its final status.
>
> fixup_user_fault() itself does not have a good way to notice this, because
> vma->vm_page_prot is an opaque pgprot_t, so the fix needs to be somewhere
> else. Other preexisting functions, namely vmf_insert_page_mkwrite()
> as well as vmf_insert_pfn_pmd(), suggest that this has to be the driver.
> In fact, of the five vm_ops that use .pfn_mkwrite() together with .fault(),
> three are in file systems and are not buggy: all of them ultimately end
> up in dax_fault_iter(), which uses vmf_insert_page_mkwrite() to correctly
> insert the PTE.
>
> The two problematic implementations instead are both in drm code. One, in
> drm_gem_shmem_helper, was reported as a KVM regression; the other, in
> vmwgfx, was found by inspection of .pfn_mkwrite() implementors.
> Both of these use pfn-mapped regions, but there is nothing like a
> vmf_insert_pfn_mkwrite() function that they could use; the first part of
> this series thus adjusts mm.h to provide two new functions for this
> usecase---vmf_insert_pfn_prot_mkwrite() and vmf_insert_pfn_mkwrite()---and
> then teaches drm's two users of .pfn_mkwrite() to call them.
>
> This however leaves another case buggy where fixup_user_fault() is preceded
> by follow_pfnmap_start(). Most callers of follow_pfnmap_start(), seeing
> it return 0 for a PFN that is mapped read-only, would not attempt to
> call fixup_user_fault() on it, and thus the PTE would not be upgraded
> to writable.
>
> This is arguably a bug in... almost all the callers of follow_pfnmap_start(),
> but fixing it is much better achieved with a small improvement to the API;
> if follow_pfnmap_start() is told by the caller that it needs the memory
> for a write, most callers are simplified because they were doing such a check
> anyway and now just see -EFAULT. They then proceed to call fixup_user_fault()
> and everyone is happy. This is done in patch 5.
>
> To sum up:
>
> - patches 1-3 introduce the new MM API, and use it in the DRM .fault()
> callbacks to install writable PTEs in response to write faults
>
> - patch 4 is a preparatory fix in KVM, eliminating inconsistencies in the
> handling of !VM_READ and !VM_WRITE VMAs; these would return different
> error codes for a !VM_WRITE VMA depending on whether the PTE happens
> to be mapped (but with wrong permissions). This needs to be here
> because the next patch would introduce even more inconsistencies.
>
> - patch 5 moves the check for writable PTEs from follow_pfnmap_start()'s
> callers to the function itself
>
> - finally, patch 6 is a KVM addendum that will have to wait until
> the next merge window; it removes yet another inconsistency in
> KVM's handling of !VM_WRITE but is technically userspace-visible,
> and therefore it shouldn't be included in stable kernel releases
> unlike the rest.
What happened to this series? It seemed liked something semi-urgent, but AFAICT
it got silently dropped?
next prev parent reply other threads:[~2026-10-26 13:49 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-04 12:05 [PATCH v2 0/6] mm, drm: fix interaction of .pfn_mkwrite() with fixup_user_fault() Paolo Bonzini
2026-08-04 12:05 ` [PATCH v2 1/6] mm: export vmf_insert_pfn_prot_mkwrite(), change variants to inline Paolo Bonzini
2026-08-04 12:20 ` sashiko-bot
2026-08-10 19:04 ` David Hildenbrand (Arm)
2026-08-04 12:05 ` [PATCH v2 2/6] drm/shmem_helper: use vmf_insert_pfn_mkwrite() Paolo Bonzini
2026-08-04 12:30 ` sashiko-bot
2026-08-04 14:15 ` Boris Brezillon
2026-08-04 14:18 ` Boris Brezillon
2026-08-04 14:34 ` Paolo Bonzini
2026-08-04 14:42 ` Boris Brezillon
2026-08-05 6:08 ` Paolo Bonzini
2026-08-05 8:34 ` Boris Brezillon
2026-08-04 12:05 ` [PATCH v2 3/6] drm/ttm, drm/vmwgfx: directly create writable PTEs when mkwrite is in use Paolo Bonzini
2026-08-04 12:21 ` sashiko-bot
2026-08-04 12:47 ` Paolo Bonzini
2026-08-06 23:32 ` Peter Xu
2026-08-10 10:01 ` Christian König
2026-08-10 10:04 ` Paolo Bonzini
2026-08-04 12:05 ` [PATCH v2 4/6] kvm: apply VM_READ/VM_WRITE checks to all VMA types Paolo Bonzini
2026-08-04 12:23 ` sashiko-bot
2026-08-04 12:44 ` Paolo Bonzini
2026-08-04 21:15 ` Sean Christopherson
2026-08-04 12:05 ` [PATCH v2 5/6] mm: pull writability check to follow_pfnmap_start() Paolo Bonzini
2026-08-04 12:14 ` sashiko-bot
2026-08-04 12:05 ` [PATCH v2 6/6] kvm: return -EFAULT for writes to !VM_WRITE IO mappings Paolo Bonzini
2026-08-04 12:14 ` sashiko-bot
2026-08-04 21:08 ` Sean Christopherson
2026-10-26 13:49 ` Sean Christopherson [this message]
2026-10-27 11:01 ` [PATCH v2 0/6] mm, drm: fix interaction of .pfn_mkwrite() with fixup_user_fault() Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=at9ahKiYsVtH8kpU@google.com \
--to=seanjc@google.com \
--cc=alex@shazbot.org \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=boris.brezillon@collabora.com \
--cc=christian.koenig@amd.com \
--cc=david@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=fei1.li@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-s390@vger.kernel.org \
--cc=mhocko@suse.com \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=ray.huang@amd.com \
--cc=slp@redhat.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.