From: Alistair Popple <apopple@nvidia.com>
Date: Wed, 28 Sep 2022 22:01:15 +1000
Subject: mm/memory.c: fix race when faulting a device private page
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Git-commit: 16ce101db85db694a91380aa4c89b25530871d33
Patch-mainline: v6.1-rc1
References: CVE-2022-3523, bsc#1204363
[ SLE15-SP4 backport notes:
- amdkfd functions missing 'trigger' parameter do to missing SMI events
- adjust to not having split out mm/migrate_device.c
- adjust to missing page folios patches ]
Patch series "Fix several device private page reference counting issues",
v2
This series aims to fix a number of page reference counting issues in
drivers dealing with device private ZONE_DEVICE pages. These result in
use-after-free type bugs, either from accessing a struct page which no
longer exists because it has been removed or accessing fields within the
struct page which are no longer valid because the page has been freed.
During normal usage it is unlikely these will cause any problems. However
without these fixes it is possible to crash the kernel from userspace.
These crashes can be triggered either by unloading the kernel module or
unbinding the device from the driver prior to a userspace task exiting.
In modules such as Nouveau it is also possible to trigger some of these
issues by explicitly closing the device file-descriptor prior to the task
exiting and then accessing device private memory.
This involves some minor changes to both PowerPC and AMD GPU code.
Unfortunately I lack hardware to test either of those so any help there
would be appreciated. The changes mimic what is done in for both Nouveau
and hmm-tests though so I doubt they will cause problems.
This patch (of 8):
When the CPU tries to access a device private page the migrate_to_ram()
callback associated with the pgmap for the page is called. However no
reference is taken on the faulting page. Therefore a concurrent migration
of the device private page can free the page and possibly the underlying
pgmap. This results in a race which can crash the kernel due to the
migrate_to_ram() function pointer becoming invalid. It also means drivers
can't reliably read the zone_device_data field because the page may have
been freed with memunmap_pages().
Close the race by getting a reference on the page while holding the ptl to
ensure it has not been freed. Unfortunately the elevated reference count
will cause the migration required to handle the fault to fail. To avoid
this failure pass the faulting page into the migrate_vma functions so that
if an elevated reference count is found it can be checked to see if it's
expected or not.
[mpe@ellerman.id.au: fix build]
Link: https://lkml.kernel.org/r/87fsgbf3gh.fsf@mpe.ellerman.id.au
Link: https://lkml.kernel.org/r/cover.60659b549d8509ddecafad4f498ee7f03bb23c69.1664366292.git-series.apopple@nvidia.com
Link: https://lkml.kernel.org/r/d3e813178a59e565e8d78d9b9a4e2562f6494f90.1664366292.git-series.apopple@nvidia.com
Signed-off-by: Alistair Popple <apopple@nvidia.com>
Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
Cc: Jason Gunthorpe <jgg@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Ralph Campbell <rcampbell@nvidia.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Alex Sierra <alex.sierra@amd.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: "Huang, Ying" <ying.huang@intel.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Yang Shi <shy828301@gmail.com>
Cc: Zi Yan <ziy@nvidia.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
arch/powerpc/kvm/book3s_hv_uvmem.c | 19 +++++++++++--------
drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 18 +++++++++++-------
drivers/gpu/drm/amd/amdkfd/kfd_migrate.h | 3 ++-
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 9 +++++----
include/linux/migrate.h | 9 +++++++++
lib/test_hmm.c | 5 +++--
mm/memory.c | 16 +++++++++++++++-
mm/migrate.c | 30 ++++++++++++++++++++++--------
8 files changed, 78 insertions(+), 31 deletions(-)
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -507,10 +507,10 @@ unsigned long kvmppc_h_svm_init_start(st
static int __kvmppc_svm_page_out(struct vm_area_struct *vma,
unsigned long start,
unsigned long end, unsigned long page_shift,
- struct kvm *kvm, unsigned long gpa)
+ struct kvm *kvm, unsigned long gpa, struct page *fault_page)
{
unsigned long src_pfn, dst_pfn = 0;
- struct migrate_vma mig;
+ struct migrate_vma mig = { 0 };
struct page *dpage, *spage;
struct kvmppc_uvmem_page_pvt *pvt;
unsigned long pfn;
@@ -524,6 +524,7 @@ static int __kvmppc_svm_page_out(struct
mig.dst = &dst_pfn;
mig.pgmap_owner = &kvmppc_uvmem_pgmap;
mig.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
+ mig.fault_page = fault_page;
/* The requested page is already paged-out, nothing to do */
if (!kvmppc_gfn_is_uvmem_pfn(gpa >> page_shift, kvm, NULL))
@@ -579,12 +580,14 @@ out_finalize:
static inline int kvmppc_svm_page_out(struct vm_area_struct *vma,
unsigned long start, unsigned long end,
unsigned long page_shift,
- struct kvm *kvm, unsigned long gpa)
+ struct kvm *kvm, unsigned long gpa,
+ struct page *fault_page)
{
int ret;
mutex_lock(&kvm->arch.uvmem_lock);
- ret = __kvmppc_svm_page_out(vma, start, end, page_shift, kvm, gpa);
+ ret = __kvmppc_svm_page_out(vma, start, end, page_shift, kvm, gpa,
+ fault_page);
mutex_unlock(&kvm->arch.uvmem_lock);
return ret;
@@ -633,7 +636,7 @@ void kvmppc_uvmem_drop_pages(const struc
pvt->remove_gfn = true;
if (__kvmppc_svm_page_out(vma, addr, addr + PAGE_SIZE,
- PAGE_SHIFT, kvm, pvt->gpa))
+ PAGE_SHIFT, kvm, pvt->gpa, NULL))
pr_err("Can't page out gpa:0x%lx addr:0x%lx\n",
pvt->gpa, addr);
} else {
@@ -736,7 +739,7 @@ static int kvmppc_svm_page_in(struct vm_
bool pagein)
{
unsigned long src_pfn, dst_pfn = 0;
- struct migrate_vma mig;
+ struct migrate_vma mig = { 0 };
struct page *spage;
unsigned long pfn;
struct page *dpage;
@@ -994,7 +997,7 @@ static vm_fault_t kvmppc_uvmem_migrate_t
if (kvmppc_svm_page_out(vmf->vma, vmf->address,
vmf->address + PAGE_SIZE, PAGE_SHIFT,
- pvt->kvm, pvt->gpa))
+ pvt->kvm, pvt->gpa, vmf->page))
return VM_FAULT_SIGBUS;
else
return 0;
@@ -1065,7 +1068,7 @@ kvmppc_h_svm_page_out(struct kvm *kvm, u
if (!vma || vma->vm_start > start || vma->vm_end < end)
goto out;
- if (!kvmppc_svm_page_out(vma, start, end, page_shift, kvm, gpa))
+ if (!kvmppc_svm_page_out(vma, start, end, page_shift, kvm, gpa, NULL))
ret = H_SUCCESS;
out:
mmap_read_unlock(kvm->mm);
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -380,7 +380,7 @@ svm_migrate_vma_to_vram(struct amdgpu_de
uint64_t npages = (end - start) >> PAGE_SHIFT;
struct kfd_process_device *pdd;
struct dma_fence *mfence = NULL;
- struct migrate_vma migrate;
+ struct migrate_vma migrate = { 0 };
dma_addr_t *scratch;
size_t size;
void *buf;
@@ -601,12 +601,13 @@ out_oom:
static int
svm_migrate_vma_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
- struct vm_area_struct *vma, uint64_t start, uint64_t end)
+ struct vm_area_struct *vma, uint64_t start, uint64_t end,
+ struct page *fault_page)
{
uint64_t npages = (end - start) >> PAGE_SHIFT;
struct kfd_process_device *pdd;
struct dma_fence *mfence = NULL;
- struct migrate_vma migrate;
+ struct migrate_vma migrate = { 0 };
dma_addr_t *scratch;
size_t size;
void *buf;
@@ -627,6 +628,7 @@ svm_migrate_vma_to_ram(struct amdgpu_dev
migrate.src = buf;
migrate.dst = migrate.src + npages;
+ migrate.fault_page = fault_page;
scratch = (dma_addr_t *)(migrate.dst + npages);
r = migrate_vma_setup(&migrate);
@@ -673,7 +675,8 @@ out:
* Return:
* 0 - OK, otherwise error code
*/
-int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm)
+int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
+ struct page *fault_page)
{
struct amdgpu_device *adev;
struct vm_area_struct *vma;
@@ -710,7 +713,8 @@ int svm_migrate_vram_to_ram(struct svm_r
break;
next = min(vma->vm_end, end);
- r = svm_migrate_vma_to_ram(adev, prange, vma, addr, next);
+ r = svm_migrate_vma_to_ram(adev, prange, vma, addr, next,
+ fault_page);
if (r) {
pr_debug("failed %d to migrate\n", r);
break;
@@ -749,7 +753,7 @@ svm_migrate_vram_to_vram(struct svm_rang
pr_debug("from gpu 0x%x to gpu 0x%x\n", prange->actual_loc, best_loc);
- r = svm_migrate_vram_to_ram(prange, mm);
+ r = svm_migrate_vram_to_ram(prange, mm, NULL);
if (r)
return r;
@@ -845,7 +849,7 @@ static vm_fault_t svm_migrate_to_ram(str
goto out_unlock_prange;
}
- r = svm_migrate_vram_to_ram(prange, vmf->vma->vm_mm);
+ r = svm_migrate_vram_to_ram(prange, vmf->vma->vm_mm, vmf->page);
if (r)
pr_debug("failed %d migrate svms 0x%p range 0x%p [0x%lx 0x%lx]\n",
r, prange->svms, prange, prange->start, prange->last);
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.h
@@ -42,7 +42,8 @@ enum MIGRATION_COPY_DIR {
int svm_migrate_to_vram(struct svm_range *prange, uint32_t best_loc,
struct mm_struct *mm);
-int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm);
+int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
+ struct page *fault_page);
unsigned long
svm_migrate_addr_to_pfn(struct amdgpu_device *adev, unsigned long addr);
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2551,12 +2551,13 @@ retry_write_locked:
* VRAM failed
*/
if (prange->actual_loc)
- r = svm_migrate_vram_to_ram(prange, mm);
+ r = svm_migrate_vram_to_ram(prange, mm,
+ NULL);
else
r = 0;
}
} else {
- r = svm_migrate_vram_to_ram(prange, mm);
+ r = svm_migrate_vram_to_ram(prange, mm, NULL);
}
if (r) {
pr_debug("failed %d to migrate svms %p [0x%lx 0x%lx]\n",
@@ -2858,7 +2859,7 @@ svm_range_trigger_migration(struct mm_st
return 0;
if (!best_loc) {
- r = svm_migrate_vram_to_ram(prange, mm);
+ r = svm_migrate_vram_to_ram(prange, mm, NULL);
*migrated = !r;
return r;
}
@@ -2916,7 +2917,7 @@ static void svm_range_evict_svm_bo_worke
prange->start, prange->last);
mutex_lock(&prange->migrate_mutex);
- svm_migrate_vram_to_ram(prange, svm_bo->eviction_fence->mm);
+ svm_migrate_vram_to_ram(prange, svm_bo->eviction_fence->mm, NULL);
mutex_lock(&prange->lock);
prange->svm_bo = NULL;
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -37,6 +37,9 @@ extern const char *migrate_reason_names[
#ifdef CONFIG_MIGRATION
extern void putback_movable_pages(struct list_head *l);
+int migrate_page_extra(struct address_space *mapping, struct page *newpage,
+ struct page *page, enum migrate_mode mode,
+ int extra_count);
extern int migrate_page(struct address_space *mapping,
struct page *newpage, struct page *page,
enum migrate_mode mode);
@@ -161,6 +164,12 @@ struct migrate_vma {
*/
void *pgmap_owner;
unsigned long flags;
+
+ /*
+ * Set to vmf->page if this is being called to migrate a page as part of
+ * a migrate_to_ram() callback.
+ */
+ struct page *fault_page;
};
int migrate_vma_setup(struct migrate_vma *args);
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -785,7 +785,7 @@ static int dmirror_migrate(struct dmirro
unsigned long src_pfns[64];
unsigned long dst_pfns[64];
struct dmirror_bounce bounce;
- struct migrate_vma args;
+ struct migrate_vma args = { 0 };
unsigned long next;
int ret;
@@ -1146,7 +1146,7 @@ static vm_fault_t dmirror_devmem_fault_a
static vm_fault_t dmirror_devmem_fault(struct vm_fault *vmf)
{
- struct migrate_vma args;
+ struct migrate_vma args = { 0 };
unsigned long src_pfns;
unsigned long dst_pfns;
struct page *rpage;
@@ -1169,6 +1169,7 @@ static vm_fault_t dmirror_devmem_fault(s
args.dst = &dst_pfns;
args.pgmap_owner = dmirror->mdevice;
args.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
+ args.fault_page = vmf->page;
if (migrate_vma_setup(&args))
return VM_FAULT_SIGBUS;
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3491,7 +3491,21 @@ vm_fault_t do_swap_page(struct vm_fault
ret = remove_device_exclusive_entry(vmf);
} else if (is_device_private_entry(entry)) {
vmf->page = pfn_swap_entry_to_page(entry);
- ret = vmf->page->pgmap->ops->migrate_to_ram(vmf);
+ vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
+ vmf->address, &vmf->ptl);
+ if (unlikely(!pte_same(*vmf->pte, vmf->orig_pte))) {
+ spin_unlock(vmf->ptl);
+ goto out;
+ }
+
+ /*
+ * Get a page reference while we know the page can't be
+ * freed.
+ */
+ get_page(vmf->page);
+ pte_unmap_unlock(vmf->pte, vmf->ptl);
+ vmf->page->pgmap->ops->migrate_to_ram(vmf);
+ put_page(vmf->page);
} else if (is_hwpoison_entry(entry)) {
ret = VM_FAULT_HWPOISON;
} else {
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -633,15 +633,15 @@ EXPORT_SYMBOL(migrate_page_copy);
*
* Pages are locked upon entry and exit.
*/
-int migrate_page(struct address_space *mapping,
+int migrate_page_extra(struct address_space *mapping,
struct page *newpage, struct page *page,
- enum migrate_mode mode)
+ enum migrate_mode mode, int extra_count)
{
int rc;
BUG_ON(PageWriteback(page)); /* Writeback must be complete */
- rc = migrate_page_move_mapping(mapping, newpage, page, 0);
+ rc = migrate_page_move_mapping(mapping, newpage, page, extra_count);
if (rc != MIGRATEPAGE_SUCCESS)
return rc;
@@ -652,6 +652,13 @@ int migrate_page(struct address_space *m
migrate_page_states(newpage, page);
return MIGRATEPAGE_SUCCESS;
}
+
+int migrate_page(struct address_space *mapping,
+ struct page *newpage, struct page *page,
+ enum migrate_mode mode)
+{
+ return migrate_page_extra(mapping, newpage, page, mode, 0);
+}
EXPORT_SYMBOL(migrate_page);
#ifdef CONFIG_BLOCK
@@ -2389,14 +2396,14 @@ static void migrate_vma_collect(struct m
* migrate_page_move_mapping(), except that here we allow migration of a
* ZONE_DEVICE page.
*/
-static bool migrate_vma_check_page(struct page *page)
+static bool migrate_vma_check_page(struct page *page, struct page *fault_page)
{
/*
* One extra ref because caller holds an extra reference, either from
* isolate_lru_page() for a regular page, or migrate_vma_collect() for
* a device page.
*/
- int extra = 1;
+ int extra = 1 + (page == fault_page);
/*
* FIXME support THP (transparent huge page), it is bit more complex to
@@ -2504,7 +2511,7 @@ static void migrate_vma_prepare(struct m
put_page(page);
}
- if (!migrate_vma_check_page(page)) {
+ if (!migrate_vma_check_page(page, migrate->fault_page)) {
if (remap) {
migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
migrate->cpages--;
@@ -2571,7 +2578,7 @@ static void migrate_vma_unmap(struct mig
goto restore;
}
- if (migrate_vma_check_page(page))
+ if (migrate_vma_check_page(page, migrate->fault_page))
continue;
restore:
@@ -2681,6 +2688,8 @@ int migrate_vma_setup(struct migrate_vma
return -EINVAL;
if (!args->src || !args->dst)
return -EINVAL;
+ if (args->fault_page && !is_device_private_page(args->fault_page))
+ return -EINVAL;
memset(args->src, 0, sizeof(*args->src) * nr_pages);
args->cpages = 0;
@@ -2913,7 +2922,12 @@ void migrate_vma_pages(struct migrate_vm
}
}
- r = migrate_page(mapping, newpage, page, MIGRATE_SYNC_NO_COPY);
+ if (migrate->fault_page == page)
+ r = migrate_page_extra(mapping, newpage, page,
+ MIGRATE_SYNC_NO_COPY, 1);
+ else
+ r = migrate_page(mapping, newpage, page,
+ MIGRATE_SYNC_NO_COPY);
if (r != MIGRATEPAGE_SUCCESS)
migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
}