From: Greg Kroah-Hartman Date: Fri, 31 Aug 2018 20:12:39 +0000 (-0700) Subject: 4.9-stable patches X-Git-Tag: v3.18.121~34 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c06ca5d092447d0db25a81b9759e4109381795e2;p=thirdparty%2Fkernel%2Fstable-queue.git 4.9-stable patches added patches: staging-android-ion-fix-ion_ioc_-map-share-use-after-free.patch --- diff --git a/queue-4.9/series b/queue-4.9/series index bbad47a12d9..44c36ebfd96 100644 --- a/queue-4.9/series +++ b/queue-4.9/series @@ -60,3 +60,4 @@ s390-kvm-fix-deadlock-when-killed-by-oom.patch ext4-check-for-nul-characters-in-extended-attribute-s-name.patch ext4-sysfs-print-ext4_super_block-fields-as-little-endian.patch ext4-reset-error-code-in-ext4_find_entry-in-fallback.patch +staging-android-ion-fix-ion_ioc_-map-share-use-after-free.patch diff --git a/queue-4.9/staging-android-ion-fix-ion_ioc_-map-share-use-after-free.patch b/queue-4.9/staging-android-ion-fix-ion_ioc_-map-share-use-after-free.patch new file mode 100644 index 00000000000..e89fe03497f --- /dev/null +++ b/queue-4.9/staging-android-ion-fix-ion_ioc_-map-share-use-after-free.patch @@ -0,0 +1,179 @@ +From ghackmann@android.com Fri Aug 31 13:10:56 2018 +From: Greg Hackmann +Date: Fri, 31 Aug 2018 13:06:27 -0700 +Subject: staging: android: ion: fix ION_IOC_{MAP,SHARE} use-after-free +To: linux-kernel@vger.kernel.org +Cc: Laura Abbott , Sumit Semwal , Greg Kroah-Hartman , devel@driverdev.osuosl.org, kernel-team@android.com, Greg Hackmann , stable@vger.kernel.org +Message-ID: <20180831200627.59712-1-ghackmann@google.com> + +From: Greg Hackmann + +This patch is 4.9.y only. Kernels 4.12 and later are unaffected, since +all the underlying ion_handle infrastructure has been ripped out. + +The ION_IOC_{MAP,SHARE} ioctls drop and reacquire client->lock several +times while operating on one of the client's ion_handles. This creates +windows where userspace can call ION_IOC_FREE on the same client with +the same handle, and effectively make the kernel drop its own reference. +For example: + +- thread A: ION_IOC_ALLOC creates an ion_handle with refcount 1 +- thread A: starts ION_IOC_MAP and increments the refcount to 2 +- thread B: ION_IOC_FREE decrements the refcount to 1 +- thread B: ION_IOC_FREE decrements the refcount to 0 and frees the + handle +- thread A: continues ION_IOC_MAP with a dangling ion_handle * to + freed memory + +Fix this by holding client->lock for the duration of +ION_IOC_{MAP,SHARE}, preventing the concurrent ION_IOC_FREE. Also +remove ion_handle_get_by_id(), since there's literally no way to use it +safely. + +Cc: stable@vger.kernel.org # v4.11- +Signed-off-by: Greg Hackmann +Signed-off-by: Greg Kroah-Hartman +--- + drivers/staging/android/ion/ion-ioctl.c | 12 +++++--- + drivers/staging/android/ion/ion.c | 48 +++++++++++++++++++------------- + drivers/staging/android/ion/ion_priv.h | 6 ++-- + 3 files changed, 40 insertions(+), 26 deletions(-) + +--- a/drivers/staging/android/ion/ion-ioctl.c ++++ b/drivers/staging/android/ion/ion-ioctl.c +@@ -128,11 +128,15 @@ long ion_ioctl(struct file *filp, unsign + { + struct ion_handle *handle; + +- handle = ion_handle_get_by_id(client, data.handle.handle); +- if (IS_ERR(handle)) ++ mutex_lock(&client->lock); ++ handle = ion_handle_get_by_id_nolock(client, data.handle.handle); ++ if (IS_ERR(handle)) { ++ mutex_unlock(&client->lock); + return PTR_ERR(handle); +- data.fd.fd = ion_share_dma_buf_fd(client, handle); +- ion_handle_put(handle); ++ } ++ data.fd.fd = ion_share_dma_buf_fd_nolock(client, handle); ++ ion_handle_put_nolock(handle); ++ mutex_unlock(&client->lock); + if (data.fd.fd < 0) + ret = data.fd.fd; + break; +--- a/drivers/staging/android/ion/ion.c ++++ b/drivers/staging/android/ion/ion.c +@@ -352,18 +352,6 @@ struct ion_handle *ion_handle_get_by_id_ + return handle ? handle : ERR_PTR(-EINVAL); + } + +-struct ion_handle *ion_handle_get_by_id(struct ion_client *client, +- int id) +-{ +- struct ion_handle *handle; +- +- mutex_lock(&client->lock); +- handle = ion_handle_get_by_id_nolock(client, id); +- mutex_unlock(&client->lock); +- +- return handle; +-} +- + static bool ion_handle_validate(struct ion_client *client, + struct ion_handle *handle) + { +@@ -1029,24 +1017,28 @@ static struct dma_buf_ops dma_buf_ops = + .kunmap = ion_dma_buf_kunmap, + }; + +-struct dma_buf *ion_share_dma_buf(struct ion_client *client, +- struct ion_handle *handle) ++static struct dma_buf *__ion_share_dma_buf(struct ion_client *client, ++ struct ion_handle *handle, ++ bool lock_client) + { + DEFINE_DMA_BUF_EXPORT_INFO(exp_info); + struct ion_buffer *buffer; + struct dma_buf *dmabuf; + bool valid_handle; + +- mutex_lock(&client->lock); ++ if (lock_client) ++ mutex_lock(&client->lock); + valid_handle = ion_handle_validate(client, handle); + if (!valid_handle) { + WARN(1, "%s: invalid handle passed to share.\n", __func__); +- mutex_unlock(&client->lock); ++ if (lock_client) ++ mutex_unlock(&client->lock); + return ERR_PTR(-EINVAL); + } + buffer = handle->buffer; + ion_buffer_get(buffer); +- mutex_unlock(&client->lock); ++ if (lock_client) ++ mutex_unlock(&client->lock); + + exp_info.ops = &dma_buf_ops; + exp_info.size = buffer->size; +@@ -1061,14 +1053,21 @@ struct dma_buf *ion_share_dma_buf(struct + + return dmabuf; + } ++ ++struct dma_buf *ion_share_dma_buf(struct ion_client *client, ++ struct ion_handle *handle) ++{ ++ return __ion_share_dma_buf(client, handle, true); ++} + EXPORT_SYMBOL(ion_share_dma_buf); + +-int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle) ++static int __ion_share_dma_buf_fd(struct ion_client *client, ++ struct ion_handle *handle, bool lock_client) + { + struct dma_buf *dmabuf; + int fd; + +- dmabuf = ion_share_dma_buf(client, handle); ++ dmabuf = __ion_share_dma_buf(client, handle, lock_client); + if (IS_ERR(dmabuf)) + return PTR_ERR(dmabuf); + +@@ -1078,8 +1077,19 @@ int ion_share_dma_buf_fd(struct ion_clie + + return fd; + } ++ ++int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle) ++{ ++ return __ion_share_dma_buf_fd(client, handle, true); ++} + EXPORT_SYMBOL(ion_share_dma_buf_fd); + ++int ion_share_dma_buf_fd_nolock(struct ion_client *client, ++ struct ion_handle *handle) ++{ ++ return __ion_share_dma_buf_fd(client, handle, false); ++} ++ + struct ion_handle *ion_import_dma_buf(struct ion_client *client, + struct dma_buf *dmabuf) + { +--- a/drivers/staging/android/ion/ion_priv.h ++++ b/drivers/staging/android/ion/ion_priv.h +@@ -463,11 +463,11 @@ void ion_free_nolock(struct ion_client * + + int ion_handle_put_nolock(struct ion_handle *handle); + +-struct ion_handle *ion_handle_get_by_id(struct ion_client *client, +- int id); +- + int ion_handle_put(struct ion_handle *handle); + + int ion_query_heaps(struct ion_client *client, struct ion_heap_query *query); + ++int ion_share_dma_buf_fd_nolock(struct ion_client *client, ++ struct ion_handle *handle); ++ + #endif /* _ION_PRIV_H */