From 4e0b8d44f6f361518bf15424ccd8ae364d446d4a Mon Sep 17 00:00:00 2001 From: Deepak Kumar Date: Sat, 12 Jun 2021 20:16:57 +0530 Subject: [PATCH] msm: kgsl: Keep private intact until last refcount is put Currently, we are partially freeing private once last file is closed but closing last file doesn't necessarily means that all refcount on private is also put. This results in having a partially initialized private struct which is not yet freed and also can't be re-used. As a result if an application is just opening and closing kgsl device in a loop we will end up allocating and initializing a new private every time. This also allows application to mis-use this and create more than allowed limit of 200 contexts as they can use newly created private to create new context every time. To fix this, keep private intact until last refcount is put. This ensures we can re-use same private if same pid is doing kgsl open and close repeatedly and also ensures that an app can't breach 200 context limit. Change-Id: Ia1d1071275c7691a3d50bc15477b71387c5806d6 Signed-off-by: Deepak Kumar --- drivers/gpu/msm/kgsl.c | 53 +++++++++++++++----------------- drivers/gpu/msm/kgsl_sharedmem.c | 25 ++++++--------- 2 files changed, 34 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/msm/kgsl.c b/drivers/gpu/msm/kgsl.c index fac02e36999b..2388ccd48b70 100644 --- a/drivers/gpu/msm/kgsl.c +++ b/drivers/gpu/msm/kgsl.c @@ -869,11 +869,25 @@ static void kgsl_destroy_process_private(struct kref *kref) struct kgsl_process_private *private = container_of(kref, struct kgsl_process_private, refcount); + mutex_lock(&kgsl_driver.process_mutex); + debugfs_remove_recursive(private->debug_root); + kgsl_process_uninit_sysfs(private); + + /* When using global pagetables, do not detach global pagetable */ + if (private->pagetable->name != KGSL_MMU_GLOBAL_PT) + kgsl_mmu_detach_pagetable(private->pagetable); + + /* Remove the process struct from the master list */ + write_lock(&kgsl_driver.proclist_lock); + list_del(&private->list); + write_unlock(&kgsl_driver.proclist_lock); + mutex_unlock(&kgsl_driver.process_mutex); + put_pid(private->pid); idr_destroy(&private->mem_idr); idr_destroy(&private->syncsource_idr); - /* When using global pagetables, do not detach global pagetable */ + /* When using global pagetables, do not put global pagetable */ if (private->pagetable->name != KGSL_MMU_GLOBAL_PT) kgsl_mmu_putpagetable(private->pagetable); @@ -962,8 +976,15 @@ static struct kgsl_process_private *kgsl_process_private_new( kfree(private); private = ERR_PTR(err); + return private; } + kgsl_process_init_sysfs(device, private); + kgsl_process_init_debugfs(private); + write_lock(&kgsl_driver.proclist_lock); + list_add(&private->list, &kgsl_driver.process_list); + write_unlock(&kgsl_driver.proclist_lock); + return private; } @@ -1008,8 +1029,8 @@ static void kgsl_process_private_close(struct kgsl_device_private *dev_priv, } /* - * If this is the last file on the process take down the debug - * directories and garbage collect any outstanding resources + * If this is the last file on the process garbage collect + * any outstanding resources */ process_release_memory(private); @@ -1017,18 +1038,6 @@ static void kgsl_process_private_close(struct kgsl_device_private *dev_priv, /* Release all syncsource objects from process private */ kgsl_syncsource_process_release_syncsources(private); - debugfs_remove_recursive(private->debug_root); - kgsl_process_uninit_sysfs(private); - - /* When using global pagetables, do not detach global pagetable */ - if (private->pagetable->name != KGSL_MMU_GLOBAL_PT) - kgsl_mmu_detach_pagetable(private->pagetable); - - /* Remove the process struct from the master list */ - write_lock(&kgsl_driver.proclist_lock); - list_del(&private->list); - write_unlock(&kgsl_driver.proclist_lock); - mutex_unlock(&kgsl_driver.process_mutex); kgsl_process_private_put(private); @@ -1046,19 +1055,7 @@ static struct kgsl_process_private *kgsl_process_private_open( if (IS_ERR(private)) goto done; - /* - * If this is a new process create the debug directories and add it to - * the process list - */ - - if (private->fd_count++ == 0) { - kgsl_process_init_sysfs(device, private); - kgsl_process_init_debugfs(private); - - write_lock(&kgsl_driver.proclist_lock); - list_add(&private->list, &kgsl_driver.process_list); - write_unlock(&kgsl_driver.proclist_lock); - } + private->fd_count++; done: mutex_unlock(&kgsl_driver.process_mutex); diff --git a/drivers/gpu/msm/kgsl_sharedmem.c b/drivers/gpu/msm/kgsl_sharedmem.c index 8bb7bd4aadc7..599c5b61e9f8 100644 --- a/drivers/gpu/msm/kgsl_sharedmem.c +++ b/drivers/gpu/msm/kgsl_sharedmem.c @@ -202,10 +202,10 @@ static ssize_t mem_entry_sysfs_show(struct kobject *kobj, ssize_t ret; /* - * kgsl_process_init_sysfs takes a refcount to the process_private, - * which is put when the kobj is released. This implies that priv will - * not be freed until this function completes, and no further locking - * is needed. + * sysfs_remove_file waits for reads to complete before the node is + * deleted and process private is freed only once kobj is released. + * This implies that priv will not be freed until this function + * completes, and no further locking is needed. */ priv = kobj ? container_of(kobj, struct kgsl_process_private, kobj) : NULL; @@ -228,10 +228,10 @@ static ssize_t memtype_sysfs_show(struct kobject *kobj, int id = 0; /* - * kgsl_process_init_sysfs takes a refcount to the process_private, - * which is put when the kobj is released. This implies that priv will - * not be freed until this function completes, and no further locking - * is needed. + * sysfs_remove_file waits for reads to complete before the node is + * deleted and process private is freed only once kobj is released. + * This implies that priv will not be freed until this function + * completes, and no further locking is needed. */ priv = container_of(kobj, struct kgsl_process_private, kobj_memtype); memtype = container_of(attr, struct kgsl_memtype, attr); @@ -261,13 +261,9 @@ static ssize_t memtype_sysfs_show(struct kobject *kobj, return scnprintf(buf, PAGE_SIZE, "%llu\n", size); } +/* Dummy release function - we have nothing to do here */ static void mem_entry_release(struct kobject *kobj) { - struct kgsl_process_private *priv; - - priv = container_of(kobj, struct kgsl_process_private, kobj); - /* Put the refcount we got in kgsl_process_init_sysfs */ - kgsl_process_private_put(priv); } static const struct sysfs_ops mem_entry_sysfs_ops = { @@ -329,9 +325,6 @@ void kgsl_process_init_sysfs(struct kgsl_device *device, { int i; - /* Keep private valid until the sysfs enries are removed. */ - kgsl_process_private_get(private); - if (kobject_init_and_add(&private->kobj, &ktype_mem_entry, kgsl_driver.prockobj, "%d", pid_nr(private->pid))) { dev_err(device->dev, "Unable to add sysfs for process %d\n",