From patchwork Fri Dec 13 22:24:59 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Stultz X-Patchwork-Id: 22450 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ob0-f199.google.com (mail-ob0-f199.google.com [209.85.214.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 80A7723FBA for ; Fri, 13 Dec 2013 22:28:21 +0000 (UTC) Received: by mail-ob0-f199.google.com with SMTP id gq1sf8721402obb.2 for ; Fri, 13 Dec 2013 14:28:21 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:from:to:cc:subject :date:message-id:in-reply-to:references:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :list-post:list-help:list-archive:list-unsubscribe; bh=/n5D2SmYC8/TytMzTdJyb2K2yBZOiwt33sxxfT6yL50=; b=byAD91FhpgkGtaCB2OWrWz2/aIKXChm9Ysue46m6EZNdEfHbygDCHQXot9QDNjjl4U Jd8XiVpneTQ04sw/XHKMAgmREHxG5NFiLv24CqVJNi5JR/4349AzyIcX2pYgq+Zozkoc HBD96oXcmuvsTvTswdbJlFgAtM0K7S6PDPD/h6kyy1wv2cAtDzjCPYHqMl6YbMmCzOxV zszN6xl1vKG0NKWJm4kE187Mck/pugEnlSl+W4GREgAk8lZ4Fri4t1K1DwvYG22ywmy+ 938wUWXkQyglOWdxNwM15GUy0F3DxrQ2jaHPmmAPeoBA0cEpwI0GDAJA+LYGyZggoD2T VEpQ== X-Gm-Message-State: ALoCoQkspoESuuSYhl8S97nNkEjqUhIEY1OKYmsDBLxlAiDSpe3UhuLIhGTFWMAeWxBd5vN7b8er X-Received: by 10.182.161.105 with SMTP id xr9mr1925303obb.31.1386973701144; Fri, 13 Dec 2013 14:28:21 -0800 (PST) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.12.46 with SMTP id v14ls1146544qeb.31.gmail; Fri, 13 Dec 2013 14:28:21 -0800 (PST) X-Received: by 10.220.58.1 with SMTP id e1mr2268714vch.0.1386973700982; Fri, 13 Dec 2013 14:28:20 -0800 (PST) Received: from mail-vb0-f48.google.com (mail-vb0-f48.google.com [209.85.212.48]) by mx.google.com with ESMTPS id er6si1194973vdc.62.2013.12.13.14.28.20 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 13 Dec 2013 14:28:20 -0800 (PST) Received-SPF: neutral (google.com: 209.85.212.48 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.212.48; Received: by mail-vb0-f48.google.com with SMTP id f13so1704198vbg.7 for ; Fri, 13 Dec 2013 14:28:20 -0800 (PST) X-Received: by 10.220.199.5 with SMTP id eq5mr2274878vcb.16.1386973700810; Fri, 13 Dec 2013 14:28:20 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp73624vcz; Fri, 13 Dec 2013 14:28:20 -0800 (PST) X-Received: by 10.66.129.169 with SMTP id nx9mr6012799pab.130.1386973699979; Fri, 13 Dec 2013 14:28:19 -0800 (PST) Received: from mail-pd0-f175.google.com (mail-pd0-f175.google.com [209.85.192.175]) by mx.google.com with ESMTPS id ya10si2488554pab.269.2013.12.13.14.28.19 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 13 Dec 2013 14:28:19 -0800 (PST) Received-SPF: neutral (google.com: 209.85.192.175 is neither permitted nor denied by best guess record for domain of john.stultz@linaro.org) client-ip=209.85.192.175; Received: by mail-pd0-f175.google.com with SMTP id w10so3021749pde.34 for ; Fri, 13 Dec 2013 14:28:19 -0800 (PST) X-Received: by 10.68.200.129 with SMTP id js1mr6193503pbc.14.1386973699562; Fri, 13 Dec 2013 14:28:19 -0800 (PST) Received: from localhost.localdomain (c-67-170-153-23.hsd1.or.comcast.net. [67.170.153.23]) by mx.google.com with ESMTPSA id qz9sm7457908pbc.3.2013.12.13.14.28.16 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 13 Dec 2013 14:28:17 -0800 (PST) From: John Stultz To: LKML Cc: Greg KH , Android Kernel Team , Sumit Semwal , Jesse Barker , Colin Cross , John Stultz Subject: [PATCH 085/115] ion: hold reference to handle after ion_uhandle_get Date: Fri, 13 Dec 2013 14:24:59 -0800 Message-Id: <1386973529-4884-86-git-send-email-john.stultz@linaro.org> X-Mailer: git-send-email 1.8.3.2 In-Reply-To: <1386973529-4884-1-git-send-email-john.stultz@linaro.org> References: <1386973529-4884-1-git-send-email-john.stultz@linaro.org> X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: john.stultz@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.212.48 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , From: Colin Cross commit 1262ab1846cf76f7549c66ef709120dbfbe6d49f (ion: replace userspace handle cookies with idr) broke the locking in ion. The ION_IOC_FREE and ION_IOC_MAP ioctls were relying on ion_handle_validate to detect the case where a call raced with another ION_IOC_FREE which may have freed the struct ion_handle. Rename ion_uhandle_get to ion_handle_get_by_id, and have it take the client lock and return with an extra reference to the handle. Make each caller put its reference once it is done with the handle. Also modify users of ion_handle_validate to continue to hold the client lock after calling ion_handle_validate until they are done with the handle, and warn if ion_handle_validate is called without the client lock held. Signed-off-by: Colin Cross Signed-off-by: John Stultz --- drivers/staging/android/ion/ion.c | 56 ++++++++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index d97117f..cf9fc78 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -383,7 +383,14 @@ static void ion_handle_get(struct ion_handle *handle) static int ion_handle_put(struct ion_handle *handle) { - return kref_put(&handle->ref, ion_handle_destroy); + struct ion_client *client = handle->client; + int ret; + + mutex_lock(&client->lock); + ret = kref_put(&handle->ref, ion_handle_destroy); + mutex_unlock(&client->lock); + + return ret; } static struct ion_handle *ion_handle_lookup(struct ion_client *client, @@ -403,14 +410,24 @@ static struct ion_handle *ion_handle_lookup(struct ion_client *client, return ERR_PTR(-EINVAL); } -static struct ion_handle *ion_uhandle_get(struct ion_client *client, int id) +static struct ion_handle *ion_handle_get_by_id(struct ion_client *client, + int id) { - return idr_find(&client->idr, id); + struct ion_handle *handle; + + mutex_lock(&client->lock); + handle = idr_find(&client->idr, id); + if (handle) + ion_handle_get(handle); + mutex_unlock(&client->lock); + + return handle ? handle : ERR_PTR(-EINVAL); } static bool ion_handle_validate(struct ion_client *client, struct ion_handle *handle) { - return (ion_uhandle_get(client, handle->id) == handle); + WARN_ON(!mutex_is_locked(&client->lock)); + return (idr_find(&client->idr, handle->id) == handle); } static int ion_handle_add(struct ion_client *client, struct ion_handle *handle) @@ -503,11 +520,11 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, mutex_lock(&client->lock); ret = ion_handle_add(client, handle); + mutex_unlock(&client->lock); if (ret) { ion_handle_put(handle); handle = ERR_PTR(ret); } - mutex_unlock(&client->lock); return handle; } @@ -527,8 +544,8 @@ void ion_free(struct ion_client *client, struct ion_handle *handle) mutex_unlock(&client->lock); return; } - ion_handle_put(handle); mutex_unlock(&client->lock); + ion_handle_put(handle); } EXPORT_SYMBOL(ion_free); @@ -1021,14 +1038,15 @@ struct dma_buf *ion_share_dma_buf(struct ion_client *client, mutex_lock(&client->lock); valid_handle = ion_handle_validate(client, handle); - mutex_unlock(&client->lock); if (!valid_handle) { WARN(1, "%s: invalid handle passed to share.\n", __func__); + mutex_unlock(&client->lock); return ERR_PTR(-EINVAL); } - buffer = handle->buffer; ion_buffer_get(buffer); + mutex_unlock(&client->lock); + dmabuf = dma_buf_export(buffer, &dma_buf_ops, buffer->size, O_RDWR); if (IS_ERR(dmabuf)) { ion_buffer_put(buffer); @@ -1081,18 +1099,24 @@ struct ion_handle *ion_import_dma_buf(struct ion_client *client, int fd) handle = ion_handle_lookup(client, buffer); if (!IS_ERR(handle)) { ion_handle_get(handle); + mutex_unlock(&client->lock); goto end; } + mutex_unlock(&client->lock); + handle = ion_handle_create(client, buffer); if (IS_ERR(handle)) goto end; + + mutex_lock(&client->lock); ret = ion_handle_add(client, handle); + mutex_unlock(&client->lock); if (ret) { ion_handle_put(handle); handle = ERR_PTR(ret); } + end: - mutex_unlock(&client->lock); dma_buf_put(dmabuf); return handle; } @@ -1156,12 +1180,11 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) if (copy_from_user(&data, (void __user *)arg, sizeof(struct ion_handle_data))) return -EFAULT; - mutex_lock(&client->lock); - handle = ion_uhandle_get(client, data.handle); - mutex_unlock(&client->lock); - if (!handle) - return -EINVAL; + handle = ion_handle_get_by_id(client, data.handle); + if (IS_ERR(handle)) + return PTR_ERR(handle); ion_free(client, handle); + ion_handle_put(handle); break; } case ION_IOC_SHARE: @@ -1172,8 +1195,11 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) if (copy_from_user(&data, (void __user *)arg, sizeof(data))) return -EFAULT; - handle = ion_uhandle_get(client, data.handle); + handle = ion_handle_get_by_id(client, data.handle); + if (IS_ERR(handle)) + return PTR_ERR(handle); data.fd = ion_share_dma_buf_fd(client, handle); + ion_handle_put(handle); if (copy_to_user((void __user *)arg, &data, sizeof(data))) return -EFAULT; if (data.fd < 0)