diff options
author | Suzanne Candanedo <suzanne.candanedo@arm.com> | 2022-12-12 15:54:52 +0000 |
---|---|---|
committer | Guus Sliepen <gsliepen@google.com> | 2023-01-10 11:16:21 +0000 |
commit | 422aa1fad7e63f16000ffb9303e816b54ef3d8ca (patch) | |
tree | 68e232a052fb107520f41c1a24cede3e46622013 /mali_kbase/mali_kbase_mem.c | |
parent | 0483baae8f0ccdb93a8345200dd1ab74f16b27e8 (diff) | |
download | gpu-422aa1fad7e63f16000ffb9303e816b54ef3d8ca.tar.gz |
MIDCET-4324/GPUCORE-35490: Prevent incorrect clearing of no user free property
Our code didn't check if KBASE_REG_NO_USER_FREE was already set
in two code paths (in mali_kbase_csf_tiler_heap.c and
mali_kbase_csf.c). This could be used to maliciously clear
that KBASE_REG_NO_USER_FREE flag.
We add a new refcount for this no user free property, replacing the
KBASE_REG_NO_USER_FREE flag, as a stopgap solution.
In addition to this:
- fix a possible race condition in JIT alloc and tiler
heap init where another thread could take a no user free
reference while the DONT_NEED flag not yet being set
- fix another issue in JIT alloc where reg->flags
was updated without taking the appropriate lock
- move the no user free decref to remove_queue
to clean up context termination code
- refactor memory helpers
Also includes:
- GPUCORE-35469: Fix list del corruption issue in shrinker callback
- GPUCORE-35221 Defer the freeing of VA regions in the chunked tiler heap shrinker callback
- GPUCORE-35499: Fix GROUP_SUSPEND kcpu suspend handling to prevent UAF
- GPUCORE-35268: Fix UAF due to use of MEM_FLAGS_CHANGE ioctl for JIT allocs
(cherry picked from commit 7a1dc910a6a8c9c5aa06677c936c8ad6e9c369ab)
Bug: 260123539
Provenance: https://code.ipdelivery.arm.com/c/GPU/mali-ddk/+/4801
Change-Id: I7e2349b135e61054f567bdf0577d27eb224d2b12
Diffstat (limited to 'mali_kbase/mali_kbase_mem.c')
-rw-r--r-- | mali_kbase/mali_kbase_mem.c | 97 |
1 files changed, 87 insertions, 10 deletions
diff --git a/mali_kbase/mali_kbase_mem.c b/mali_kbase/mali_kbase_mem.c index fcbaf2b..1526225 100644 --- a/mali_kbase/mali_kbase_mem.c +++ b/mali_kbase/mali_kbase_mem.c @@ -1550,6 +1550,7 @@ struct kbase_va_region *kbase_alloc_free_region(struct rb_root *rbtree, return NULL; new_reg->va_refcnt = 1; + new_reg->no_user_free_refcnt = 0; new_reg->cpu_alloc = NULL; /* no alloc bound yet */ new_reg->gpu_alloc = NULL; /* no alloc bound yet */ new_reg->rbtree = rbtree; @@ -2184,11 +2185,30 @@ int kbase_mem_free_region(struct kbase_context *kctx, struct kbase_va_region *re __func__, (void *)reg, (void *)kctx); lockdep_assert_held(&kctx->reg_lock); - if (reg->flags & KBASE_REG_NO_USER_FREE) { + if (kbase_va_region_is_no_user_free(kctx, reg)) { dev_warn(kctx->kbdev->dev, "Attempt to free GPU memory whose freeing by user space is forbidden!\n"); return -EINVAL; } + /* If a region has been made evictable then we must unmake it + * before trying to free it. + * If the memory hasn't been reclaimed it will be unmapped and freed + * below, if it has been reclaimed then the operations below are no-ops. + */ + if (reg->flags & KBASE_REG_DONT_NEED) { + WARN_ON(reg->cpu_alloc->type != KBASE_MEM_TYPE_NATIVE); + mutex_lock(&kctx->jit_evict_lock); + /* Unlink the physical allocation before unmaking it evictable so + * that the allocation isn't grown back to its last backed size + * as we're going to unmap it anyway. + */ + reg->cpu_alloc->reg = NULL; + if (reg->cpu_alloc != reg->gpu_alloc) + reg->gpu_alloc->reg = NULL; + mutex_unlock(&kctx->jit_evict_lock); + kbase_mem_evictable_unmake(reg->gpu_alloc); + } + err = kbase_gpu_munmap(kctx, reg); if (err) { dev_warn(kctx->kbdev->dev, "Could not unmap from the GPU...\n"); @@ -2384,8 +2404,11 @@ int kbase_update_region_flags(struct kbase_context *kctx, if (flags & BASEP_MEM_PERMANENT_KERNEL_MAPPING) reg->flags |= KBASE_REG_PERMANENT_KERNEL_MAPPING; - if (flags & BASEP_MEM_NO_USER_FREE) - reg->flags |= KBASE_REG_NO_USER_FREE; + if (flags & BASEP_MEM_NO_USER_FREE) { + kbase_gpu_vm_lock(kctx); + kbase_va_region_no_user_free_get(kctx, reg); + kbase_gpu_vm_unlock(kctx); + } if (flags & BASE_MEM_GPU_VA_SAME_4GB_PAGE) reg->flags |= KBASE_REG_GPU_VA_SAME_4GB_PAGE; @@ -3746,7 +3769,15 @@ static void kbase_jit_destroy_worker(struct work_struct *work) mutex_unlock(&kctx->jit_evict_lock); kbase_gpu_vm_lock(kctx); - reg->flags &= ~KBASE_REG_NO_USER_FREE; + + /* + * Incrementing the refcount is prevented on JIT regions. + * If/when this ever changes we would need to compensate + * by implementing "free on putting the last reference", + * but only for JIT regions. + */ + WARN_ON(reg->no_user_free_refcnt > 1); + kbase_va_region_no_user_free_put(kctx, reg); kbase_mem_free_region(kctx, reg); kbase_gpu_vm_unlock(kctx); } while (1); @@ -4473,6 +4504,29 @@ struct kbase_va_region *kbase_jit_allocate(struct kbase_context *kctx, } } + /* Similarly to tiler heap init, there is a short window of time + * where the (either recycled or newly allocated, in our case) region has + * "no user free" refcount incremented but is still missing the DONT_NEED flag, and + * doesn't yet have the ACTIVE_JIT_ALLOC flag either. Temporarily leaking the + * allocation is the least bad option that doesn't lead to a security issue down the + * line (it will eventually be cleaned up during context termination). + * + * We also need to call kbase_gpu_vm_lock regardless, as we're updating the region + * flags. + */ + kbase_gpu_vm_lock(kctx); + if (unlikely(reg->no_user_free_refcnt > 1)) { + kbase_gpu_vm_unlock(kctx); + dev_err(kctx->kbdev->dev, "JIT region has no_user_free_refcnt > 1!\n"); + + mutex_lock(&kctx->jit_evict_lock); + list_move(®->jit_node, &kctx->jit_pool_head); + mutex_unlock(&kctx->jit_evict_lock); + + reg = NULL; + goto end; + } + trace_mali_jit_alloc(reg, info->id); kctx->jit_current_allocations++; @@ -4490,6 +4544,7 @@ struct kbase_va_region *kbase_jit_allocate(struct kbase_context *kctx, kbase_jit_report_update_pressure(kctx, reg, info->va_pages, KBASE_JIT_REPORT_ON_ALLOC_OR_FREE); #endif /* MALI_JIT_PRESSURE_LIMIT_BASE */ + kbase_gpu_vm_unlock(kctx); end: for (i = 0; i != ARRAY_SIZE(prealloc_sas); ++i) @@ -4595,11 +4650,19 @@ bool kbase_jit_evict(struct kbase_context *kctx) reg = list_entry(kctx->jit_pool_head.prev, struct kbase_va_region, jit_node); list_del(®->jit_node); + list_del_init(®->gpu_alloc->evict_node); } mutex_unlock(&kctx->jit_evict_lock); if (reg) { - reg->flags &= ~KBASE_REG_NO_USER_FREE; + /* + * Incrementing the refcount is prevented on JIT regions. + * If/when this ever changes we would need to compensate + * by implementing "free on putting the last reference", + * but only for JIT regions. + */ + WARN_ON(reg->no_user_free_refcnt > 1); + kbase_va_region_no_user_free_put(kctx, reg); kbase_mem_free_region(kctx, reg); } @@ -4619,12 +4682,17 @@ void kbase_jit_term(struct kbase_context *kctx) walker = list_first_entry(&kctx->jit_pool_head, struct kbase_va_region, jit_node); list_del(&walker->jit_node); + list_del_init(&walker->gpu_alloc->evict_node); mutex_unlock(&kctx->jit_evict_lock); - /* As context is terminating, directly free the backing pages - * without unmapping them from the GPU as done in - * kbase_region_tracker_erase_rbtree(). + /* + * Incrementing the refcount is prevented on JIT regions. + * If/when this ever changes we would need to compensate + * by implementing "free on putting the last reference", + * but only for JIT regions. */ - kbase_free_alloced_region(walker); + WARN_ON(walker->no_user_free_refcnt > 1); + kbase_va_region_no_user_free_put(kctx, walker); + kbase_mem_free_region(kctx, walker); mutex_lock(&kctx->jit_evict_lock); } @@ -4633,8 +4701,17 @@ void kbase_jit_term(struct kbase_context *kctx) walker = list_first_entry(&kctx->jit_active_head, struct kbase_va_region, jit_node); list_del(&walker->jit_node); + list_del_init(&walker->gpu_alloc->evict_node); mutex_unlock(&kctx->jit_evict_lock); - kbase_free_alloced_region(walker); + /* + * Incrementing the refcount is prevented on JIT regions. + * If/when this ever changes we would need to compensate + * by implementing "free on putting the last reference", + * but only for JIT regions. + */ + WARN_ON(walker->no_user_free_refcnt > 1); + kbase_va_region_no_user_free_put(kctx, walker); + kbase_mem_free_region(kctx, walker); mutex_lock(&kctx->jit_evict_lock); } #if MALI_JIT_PRESSURE_LIMIT_BASE |