summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSuzanne Candanedo <suzanne.candanedo@arm.com>2023-04-12 11:53:49 +0100
committerGuus Sliepen <gsliepen@google.com>2023-08-30 08:58:49 +0000
commit35feb9b795cf1cd0d9a0a2edb6ade3c83040f48b (patch)
tree89f7fea3f69b8512f3bdf2c48b5c2564310482d2
parent83b03c4f316ecc92f4b64f23c024d1f2eef8e523 (diff)
downloadgpu-android-gs-bluejay-5.10-android13-qpr3.tar.gz
Kbase accounts the GPU memory allocated for a context under the memory footprint of a process, so that kernel's low memory killer or OoM killer can kill the suitable process to free up decent amount of system memory. For accounting, Kbase updates the 'MM_FILEPAGES' counter inside the 'mm_struct' corresponding to the process that created the Kbase context. To ensure 'mm_struct' can always be safely accessed, a tracking page was used which Kbase mandated to be mapped right after the opening of '/dev/mali0' file. When the mapping was closed Kbase updated the kernel counter to subtract all the GPU memory allocated so far for a context but the actual freeing of GPU memory was done later. This was usually not a problem as the mapping is closed by DDK Userspace just before the context termination. But the Userspace is allowed to close it at will. Malicious Userspace could have exploited this Kbase quirk by closing the mapping which would have mislead the OoM killer in killing the innocent processes before it gets around to the culprit process. This commit removes the use of tracking page and uses mmgrab() to take a reference on 'mm_struct' for User created Kbase context. The reference doesn't prevent the freeing of address space on process exit, it merely keeps the 'mm_struct' alive. The reference is dropped on context termination. For backward compatibility, the call from Base to create the mapping for tracking page isn't rejected by Kbase. The Base code has been updated to skip the mapping of tracking page only for newer Kbase so as to maintain forward compatibility with the older Kbase. BASE_UK_VERSION_MINOR has been incremented as the requirement to create a mapping for tracking page is relaxed by Kbase. Change-Id: I37407809d7187cb5c8fac63e6c10f72cc3bd762d Bug: 275853921 Provenance: https://code.ipdelivery.arm.com/c/GPU/mali-ddk/+/5165 (cherry picked from commit b29f4a639e0863c09e72d7e34c0a6ad57f9572a6)
-rw-r--r--mali_kbase/context/mali_kbase_context.c21
-rw-r--r--mali_kbase/mali_kbase_defs.h9
-rw-r--r--mali_kbase/mali_kbase_mem.h11
-rw-r--r--mali_kbase/mali_kbase_mem_linux.c64
4 files changed, 32 insertions, 73 deletions
diff --git a/mali_kbase/context/mali_kbase_context.c b/mali_kbase/context/mali_kbase_context.c
index 5fc1636..9467497 100644
--- a/mali_kbase/context/mali_kbase_context.c
+++ b/mali_kbase/context/mali_kbase_context.c
@@ -176,13 +176,22 @@ int kbase_context_common_init(struct kbase_context *kctx)
/* creating a context is considered a disjoint event */
kbase_disjoint_event(kctx->kbdev);
- spin_lock_init(&kctx->mm_update_lock);
kctx->process_mm = NULL;
atomic_set(&kctx->nonmapped_pages, 0);
atomic_set(&kctx->permanent_mapped_pages, 0);
kctx->tgid = current->tgid;
kctx->pid = current->pid;
+ /* Check if this is a Userspace created context */
+ if (likely(kctx->filp)) {
+ /* This merely takes a reference on the mm_struct and not on the
+ * address space and so won't block the freeing of address space
+ * on process exit.
+ */
+ mmgrab(current->mm);
+ kctx->process_mm = current->mm;
+ }
+
atomic_set(&kctx->used_pages, 0);
mutex_init(&kctx->reg_lock);
@@ -215,9 +224,12 @@ int kbase_context_common_init(struct kbase_context *kctx)
mutex_lock(&kctx->kbdev->kctx_list_lock);
err = kbase_insert_kctx_to_process(kctx);
- if (err)
+ if (err) {
dev_err(kctx->kbdev->dev,
- "(err:%d) failed to insert kctx to kbase_process\n", err);
+ "(err:%d) failed to insert kctx to kbase_process", err);
+ if (likely(kctx->filp))
+ mmdrop(kctx->process_mm);
+ }
mutex_unlock(&kctx->kbdev->kctx_list_lock);
@@ -307,6 +319,9 @@ void kbase_context_common_term(struct kbase_context *kctx)
kbase_remove_kctx_from_process(kctx);
mutex_unlock(&kctx->kbdev->kctx_list_lock);
+ if (likely(kctx->filp))
+ mmdrop(kctx->process_mm);
+
KBASE_KTRACE_ADD(kctx->kbdev, CORE_CTX_DESTROY, kctx, 0u);
}
diff --git a/mali_kbase/mali_kbase_defs.h b/mali_kbase/mali_kbase_defs.h
index c0b8d6e..7773a90 100644
--- a/mali_kbase/mali_kbase_defs.h
+++ b/mali_kbase/mali_kbase_defs.h
@@ -1644,11 +1644,13 @@ struct kbase_sub_alloc {
* is scheduled in and an atom is pulled from the context's per
* slot runnable tree in JM GPU or GPU command queue
* group is programmed on CSG slot in CSF GPU.
- * @mm_update_lock: lock used for handling of special tracking page.
* @process_mm: Pointer to the memory descriptor of the process which
* created the context. Used for accounting the physical
* pages used for GPU allocations, done for the context,
- * to the memory consumed by the process.
+ * to the memory consumed by the process. A reference is taken
+ * on this descriptor for the Userspace created contexts so that
+ * Kbase can safely access it to update the memory usage counters.
+ * The reference is dropped on context termination.
* @gpu_va_end: End address of the GPU va space (in 4KB page units)
* @running_total_tiler_heap_nr_chunks: Running total of number of chunks in all
* tiler heaps of the kbase context.
@@ -1869,8 +1871,7 @@ struct kbase_context {
atomic_t refcount;
- spinlock_t mm_update_lock;
- struct mm_struct __rcu *process_mm;
+ struct mm_struct *process_mm;
u64 gpu_va_end;
#if MALI_USE_CSF
u32 running_total_tiler_heap_nr_chunks;
diff --git a/mali_kbase/mali_kbase_mem.h b/mali_kbase/mali_kbase_mem.h
index 97d6471..4b25cb1 100644
--- a/mali_kbase/mali_kbase_mem.h
+++ b/mali_kbase/mali_kbase_mem.h
@@ -2292,8 +2292,7 @@ kbase_ctx_reg_zone_get(struct kbase_context *kctx, unsigned long zone_bits)
* kbase_mem_allow_alloc - Check if allocation of GPU memory is allowed
* @kctx: Pointer to kbase context
*
- * Don't allow the allocation of GPU memory until user space has set up the
- * tracking page (which sets kctx->process_mm) or if the ioctl has been issued
+ * Don't allow the allocation of GPU memory if the ioctl has been issued
* from the forked child process using the mali device file fd inherited from
* the parent process.
*
@@ -2301,13 +2300,7 @@ kbase_ctx_reg_zone_get(struct kbase_context *kctx, unsigned long zone_bits)
*/
static inline bool kbase_mem_allow_alloc(struct kbase_context *kctx)
{
- bool allow_alloc = true;
-
- rcu_read_lock();
- allow_alloc = (rcu_dereference(kctx->process_mm) == current->mm);
- rcu_read_unlock();
-
- return allow_alloc;
+ return (kctx->process_mm == current->mm);
}
/**
diff --git a/mali_kbase/mali_kbase_mem_linux.c b/mali_kbase/mali_kbase_mem_linux.c
index 000efc7..ac8f51d 100644
--- a/mali_kbase/mali_kbase_mem_linux.c
+++ b/mali_kbase/mali_kbase_mem_linux.c
@@ -3240,79 +3240,29 @@ static void kbasep_add_mm_counter(struct mm_struct *mm, int member, long value)
void kbasep_os_process_page_usage_update(struct kbase_context *kctx, int pages)
{
- struct mm_struct *mm;
+ struct mm_struct *mm = kctx->process_mm;
- rcu_read_lock();
- mm = rcu_dereference(kctx->process_mm);
- if (mm) {
- atomic_add(pages, &kctx->nonmapped_pages);
-#ifdef SPLIT_RSS_COUNTING
- kbasep_add_mm_counter(mm, MM_FILEPAGES, pages);
-#else
- spin_lock(&mm->page_table_lock);
- kbasep_add_mm_counter(mm, MM_FILEPAGES, pages);
- spin_unlock(&mm->page_table_lock);
-#endif
- }
- rcu_read_unlock();
-}
-
-static void kbasep_os_process_page_usage_drain(struct kbase_context *kctx)
-{
- int pages;
- struct mm_struct *mm;
-
- spin_lock(&kctx->mm_update_lock);
- mm = rcu_dereference_protected(kctx->process_mm, lockdep_is_held(&kctx->mm_update_lock));
- if (!mm) {
- spin_unlock(&kctx->mm_update_lock);
+ if (unlikely(!mm))
return;
- }
- rcu_assign_pointer(kctx->process_mm, NULL);
- spin_unlock(&kctx->mm_update_lock);
- synchronize_rcu();
-
- pages = atomic_xchg(&kctx->nonmapped_pages, 0);
+ atomic_add(pages, &kctx->nonmapped_pages);
#ifdef SPLIT_RSS_COUNTING
- kbasep_add_mm_counter(mm, MM_FILEPAGES, -pages);
+ kbasep_add_mm_counter(mm, MM_FILEPAGES, pages);
#else
spin_lock(&mm->page_table_lock);
- kbasep_add_mm_counter(mm, MM_FILEPAGES, -pages);
+ kbasep_add_mm_counter(mm, MM_FILEPAGES, pages);
spin_unlock(&mm->page_table_lock);
#endif
}
-static void kbase_special_vm_close(struct vm_area_struct *vma)
-{
- struct kbase_context *kctx;
-
- kctx = vma->vm_private_data;
- kbasep_os_process_page_usage_drain(kctx);
-}
-
-static const struct vm_operations_struct kbase_vm_special_ops = {
- .close = kbase_special_vm_close,
-};
-
static int kbase_tracking_page_setup(struct kbase_context *kctx, struct vm_area_struct *vma)
{
- /* check that this is the only tracking page */
- spin_lock(&kctx->mm_update_lock);
- if (rcu_dereference_protected(kctx->process_mm, lockdep_is_held(&kctx->mm_update_lock))) {
- spin_unlock(&kctx->mm_update_lock);
- return -EFAULT;
- }
-
- rcu_assign_pointer(kctx->process_mm, current->mm);
-
- spin_unlock(&kctx->mm_update_lock);
+ if (vma_pages(vma) != 1)
+ return -EINVAL;
/* no real access */
vma->vm_flags &= ~(VM_READ | VM_MAYREAD | VM_WRITE | VM_MAYWRITE | VM_EXEC | VM_MAYEXEC);
vma->vm_flags |= VM_DONTCOPY | VM_DONTEXPAND | VM_DONTDUMP | VM_IO;
- vma->vm_ops = &kbase_vm_special_ops;
- vma->vm_private_data = kctx;
return 0;
}