On 15.05.23 21:07, Sean Christopherson wrote:
On Sun, May 14, 2023, Lorenzo Stoakes wrote:
No invocation of get_user_pages() use the vmas parameter, so remove it.
The GUP API is confusing and caveated. Recent changes have done much to
improve that, however there is more we can do. Exporting vmas is a prime
target as the caller has to be extremely careful to preclude their use
after the mmap_lock has expired or otherwise be left with dangling
pointers.
Removing the vmas parameter focuses the GUP functions upon their primary
purpose - pinning (and outputting) pages as well as performing the actions
implied by the input flags.
This is part of a patch series aiming to remove the vmas parameter
altogether.
Suggested-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
Acked-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Acked-by: David Hildenbrand <david@xxxxxxxxxx>
Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
Acked-by: Christian K�nig <christian.koenig@xxxxxxx> (for radeon parts)
Acked-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
Signed-off-by: Lorenzo Stoakes <lstoakes@xxxxxxxxx>
---
arch/x86/kernel/cpu/sgx/ioctl.c | 2 +-
drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
drivers/misc/sgi-gru/grufault.c | 2 +-
include/linux/mm.h | 3 +--
mm/gup.c | 9 +++------
mm/gup_test.c | 5 ++---
virt/kvm/kvm_main.c | 2 +-
7 files changed, 10 insertions(+), 15 deletions(-)
Acked-by: Sean Christopherson <seanjc@xxxxxxxxxx> (KVM)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cb5c13eee193..eaa5bb8dbadc 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2477,7 +2477,7 @@ static inline int check_user_page_hwpoison(unsigned long addr)
{
int rc, flags = FOLL_HWPOISON | FOLL_WRITE;
- rc = get_user_pages(addr, 1, flags, NULL, NULL);
+ rc = get_user_pages(addr, 1, flags, NULL);
return rc == -EHWPOISON;
Unrelated to this patch, I think there's a pre-existing bug here. If gup() returns
a valid page, KVM will leak the refcount and unintentionally pin the page. That's
When passing NULL as "pages" to get_user_pages(),
__get_user_pages_locked() won't set FOLL_GET. As FOLL_PIN is also not
set, we won't be messing with the mapcount of the page.
So even if get_user_pages() returns "1", we should be fine.
Or am I misunderstanding your concern? At least hva_to_pfn_slow() most
certainly didn't return "1" if we end up calling
check_user_page_hwpoison(), so nothing would have been pinned there as well.
--
Thanks,
David / dhildenb
