From 9ae71bc3a542f68ea93c4eff01a41201ee6d9402 Mon Sep 17 00:00:00 2001 From: Divya Ponnusamy Date: Fri, 6 May 2016 13:24:37 -0600 Subject: msm: kgsl: Avoid race condition in ioctl_syncsource_destroy If the ioctl syncsource_destroy is accessed by parallel threads, where the spinlock is acquired by threads after getting syncsource, then the simultaneous processes try to remove the already destroyed syncsource->refcount by the first thread that acquires this spinlock. This leads to race condition while removing syncsource->idr. Avoid separate lock inside getting syncsource, instead acquire spinlock before we get the syncsource in destroy ioctl so that the threads access the spinlock and operate on syncsource without use-after-free issue. Change-Id: I6add3800c40cd09f6e6e0cf2720e69059bd83cbc Signed-off-by: Divya Ponnusamy --- drivers/gpu/msm/kgsl_sync.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/msm/kgsl_sync.c b/drivers/gpu/msm/kgsl_sync.c index abbdc5d..5c3ae1b 100644 --- a/drivers/gpu/msm/kgsl_sync.c +++ b/drivers/gpu/msm/kgsl_sync.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2012-2015, The Linux Foundation. All rights reserved. +/* Copyright (c) 2012-2016, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -474,23 +474,23 @@ long kgsl_ioctl_syncsource_create(struct kgsl_device_private *dev_priv, goto out; } + kref_init(&syncsource->refcount); + syncsource->private = private; + idr_preload(GFP_KERNEL); spin_lock(&private->syncsource_lock); id = idr_alloc(&private->syncsource_idr, syncsource, 1, 0, GFP_NOWAIT); - spin_unlock(&private->syncsource_lock); - idr_preload_end(); - if (id > 0) { - kref_init(&syncsource->refcount); syncsource->id = id; - syncsource->private = private; - param->id = id; ret = 0; } else { ret = id; } + spin_unlock(&private->syncsource_lock); + idr_preload_end(); + out: if (ret) { if (syncsource && syncsource->oneshot) @@ -548,25 +548,23 @@ long kgsl_ioctl_syncsource_destroy(struct kgsl_device_private *dev_priv, { struct kgsl_syncsource_destroy *param = data; struct kgsl_syncsource *syncsource = NULL; - struct kgsl_process_private *private; - - syncsource = kgsl_syncsource_get(dev_priv->process_priv, - param->id); + struct kgsl_process_private *private = dev_priv->process_priv; - if (syncsource == NULL) - return -EINVAL; + spin_lock(&private->syncsource_lock); + syncsource = idr_find(&private->syncsource_idr, param->id); - private = syncsource->private; + if (syncsource) { + idr_remove(&private->syncsource_idr, param->id); + syncsource->id = 0; + } - spin_lock(&private->syncsource_lock); - idr_remove(&private->syncsource_idr, param->id); - syncsource->id = 0; spin_unlock(&private->syncsource_lock); + if (syncsource == NULL) + return -EINVAL; + /* put reference from syncsource creation */ kgsl_syncsource_put(syncsource); - /* put reference from getting the syncsource above */ - kgsl_syncsource_put(syncsource); return 0; } -- cgit v1.1