From eb922bef67bacbf8cfa8bff427895db427d7662f Mon Sep 17 00:00:00 2001 From: Lynus Vaz Date: Wed, 18 Feb 2015 16:49:04 +0530 Subject: [PATCH] msm: kgsl: Avoid deadlock when deleting process sysfs nodes The process mutex is held while removing the sysfs nodes, which waits for any access to finish before completion. If another thread does a read at the same time, it will wait for the process mutex which is already being held by the first thread, causing a deadlock. Get a refcount to the process private to make sure that it is valid while the sysfs nodes exist. This removes the need to hold the process mutex while reading the nodes. Issue: SEC-3014 CRs-Fixed: 796805 Change-Id: I01fd721eb6cbd7443abcc5eaf13a86844efd61b6 Signed-off-by: Lynus Vaz (cherry-picked from commit https://source.codeaurora.org/quic/la/kernel/msm-3.10/commit/?id=0e2f11c55445a4b09ac8b7946a9383830b948950) --- drivers/gpu/msm/kgsl_sharedmem.c | 44 ++++++++++++-------------------- 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/msm/kgsl_sharedmem.c b/drivers/gpu/msm/kgsl_sharedmem.c index 05577511a7a..77b2e5757ea 100644 --- a/drivers/gpu/msm/kgsl_sharedmem.c +++ b/drivers/gpu/msm/kgsl_sharedmem.c @@ -67,30 +67,6 @@ struct mem_entry_stats { mem_entry_max_show), \ } -/** - * Given a kobj, find the process structure attached to it - */ - -static struct kgsl_process_private * -_get_priv_from_kobj(struct kobject *kobj) -{ - struct kgsl_process_private *private; - unsigned int name; - - if (!kobj) - return NULL; - - if (kstrtou32(kobj->name, 0, &name)) - return NULL; - - list_for_each_entry(private, &kgsl_driver.process_list, list) { - if (private->pid == name) - return private; - } - - return NULL; -} - /** * Show the current amount of memory allocated for the given memtype */ @@ -124,15 +100,22 @@ static ssize_t mem_entry_sysfs_show(struct kobject *kobj, struct kgsl_process_private *priv; ssize_t ret; - mutex_lock(&kgsl_driver.process_mutex); - priv = _get_priv_from_kobj(kobj); + /* + * 1. sysfs_remove_file waits for reads to complete before the node + * is deleted. + * 2. kgsl_process_init_sysfs takes a refcount to the process_private, + * which is put at the end of kgsl_process_uninit_sysfs. + * These two conditions imply 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; if (priv && pattr->show) ret = pattr->show(priv, pattr->memtype, buf); else ret = -EIO; - mutex_unlock(&kgsl_driver.process_mutex); return ret; } @@ -170,6 +153,8 @@ kgsl_process_uninit_sysfs(struct kgsl_process_private *private) } kobject_put(&private->kobj); + /* Put the refcount we got in kgsl_process_init_sysfs */ + kgsl_process_private_put(private); } /** @@ -208,6 +193,11 @@ kgsl_process_init_sysfs(struct kgsl_device *device, ret = sysfs_create_file(&private->kobj, &mem_stats[i].max_attr.attr); } + + /* Keep private valid until the sysfs enries are removed. */ + if (!ret) + kgsl_process_private_get(private); + return ret; }