From 4888a345fc10e59efbdf5284879c8f2faa0bb042 Mon Sep 17 00:00:00 2001 From: binqi zhang Date: Mon, 29 Aug 2022 14:12:19 +0800 Subject: [PATCH] UVM: fix null pointer dereference [1/1] PD#SWPL-92386 Problem: 1.null pointer dereference in mua_get_meta_data 2.missing two CLs from kernel5.4 Solution: 1.add protect for potential null pointer; 2.cherry pick the following two CLs: https://scgit.amlogic.com/#/c/234215/ https://scgit.amlogic.com/#/c/217078/ Verify: AndroidT kernel5.4 ohm Change-Id: I0eb4833d75ce7e11e80fe234a21f4bd7042de8ea Signed-off-by: binqi zhang --- .../media/common/uvm/meson_uvm_allocator.c | 76 +++++++++- .../media/common/uvm/meson_uvm_allocator.h | 11 ++ drivers/media/common/uvm/meson_uvm_core.c | 137 +++++++++++++----- include/linux/amlogic/meson_uvm_core.h | 15 +- 4 files changed, 196 insertions(+), 43 deletions(-) diff --git a/drivers/media/common/uvm/meson_uvm_allocator.c b/drivers/media/common/uvm/meson_uvm_allocator.c index a9ff7abb8..681783390 100644 --- a/drivers/media/common/uvm/meson_uvm_allocator.c +++ b/drivers/media/common/uvm/meson_uvm_allocator.c @@ -39,6 +39,26 @@ module_param(mua_debug_level, int, 0644); pr_info("MUA: " fmt, ## arg); \ } while (0) +static bool mua_is_valid_dmabuf(int fd) +{ + struct dma_buf *dmabuf = NULL; + + dmabuf = dma_buf_get(fd); + if (IS_ERR_OR_NULL(dmabuf)) { + MUA_PRINTK(0, "invalid dmabuf. %s %d\n", __func__, __LINE__); + return false; + } + + if (!dmabuf_is_uvm(dmabuf)) { + MUA_PRINTK(0, "dmabuf is not uvm. %s %d\n", __func__, __LINE__); + dma_buf_put(dmabuf); + return false; + } + + dma_buf_put(dmabuf); + return true; +} + static void mua_handle_free(struct uvm_buf_obj *obj) { struct mua_buffer *buffer; @@ -365,11 +385,15 @@ static int mua_get_meta_data(int fd, ulong arg) } vfp = dmabuf_get_vframe(dmabuf); - + if (IS_ERR_OR_NULL(vfp)) { + dmabuf_put_vframe(dmabuf); + dma_buf_put(dmabuf); + return -EINVAL; + } /* check source type. */ if (!vfp->meta_data_size || vfp->meta_data_size > META_DATA_SIZE) { - MUA_PRINTK(0, "meta data size: %d is invalid.\n", + MUA_PRINTK(2, "meta data size: %d is invalid.\n", vfp->meta_data_size); dmabuf_put_vframe(dmabuf); dma_buf_put(dmabuf); @@ -429,9 +453,9 @@ static int mua_attach(int fd, int type, char *buf) return -EINVAL; } - MUA_PRINTK(1, "core_attach: type:%d buf:%s.\n", - type, buf); dmabuf = dma_buf_get(fd); + MUA_PRINTK(1, "core_attach: type:%d dmabuf:%p.\n", + type, dmabuf); if (IS_ERR_OR_NULL(dmabuf)) { MUA_PRINTK(0, "Invalid dmabuf %s %d\n", __func__, __LINE__); @@ -491,10 +515,11 @@ static long mua_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { struct mua_device *md; union uvm_ioctl_arg data; - struct dma_buf *dmabuf; + struct dma_buf *dmabuf = NULL; int pid; int ret = 0; int fd = 0; + size_t usage = 0; int alloc_buf_size = 0; md = file->private_data; @@ -532,7 +557,11 @@ static long mua_ioctl(struct file *file, unsigned int cmd, unsigned long arg) } break; case UVM_IOC_GET_INFO: - ret = meson_uvm_getinfo(data.hook_data.shared_fd, + fd = data.hook_data.shared_fd; + if (!mua_is_valid_dmabuf(fd)) + return -EINVAL; + dmabuf = dma_buf_get(fd); + ret = meson_uvm_getinfo(dmabuf, data.hook_data.mode_type, data.hook_data.data_buf); if (ret < 0) { @@ -543,7 +572,11 @@ static long mua_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return -EFAULT; break; case UVM_IOC_SET_INFO: - ret = meson_uvm_setinfo(data.hook_data.shared_fd, + fd = data.hook_data.shared_fd; + if (!mua_is_valid_dmabuf(fd)) + return -EINVAL; + dmabuf = dma_buf_get(fd); + ret = meson_uvm_setinfo(dmabuf, data.hook_data.mode_type, data.hook_data.data_buf); if (ret < 0) { @@ -607,10 +640,37 @@ static long mua_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return -EINVAL; } break; + case UVM_IOC_SET_USAGE: + fd = data.usage_data.fd; + if (!mua_is_valid_dmabuf(fd)) + return -EINVAL; + dmabuf = dma_buf_get(fd); + usage = data.usage_data.uvm_data_usage; + ret = meson_uvm_set_usage(dmabuf, usage); + if (ret < 0) { + MUA_PRINTK(1, "meson_uvm_set_usage fail.\n"); + return -EINVAL; + } + break; + case UVM_IOC_GET_USAGE: + fd = data.usage_data.fd; + if (!mua_is_valid_dmabuf(fd)) + return -EINVAL; + dmabuf = dma_buf_get(fd); + ret = meson_uvm_get_usage(dmabuf, &usage); + if (ret < 0) { + MUA_PRINTK(1, "meson_uvm_get_usage fail.\n"); + return -EINVAL; + } + data.usage_data.uvm_data_usage = usage; + if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd))) + return -EFAULT; + break; case UVM_IOC_GET_METADATA: + MUA_PRINTK(2, "%s LINE:%d.\n", __func__, __LINE__); ret = mua_get_meta_data(data.meta_data.fd, arg); if (ret < 0) { - MUA_PRINTK(0, "get meta data fail.\n"); + MUA_PRINTK(1, "get meta data fail.\n"); return -EINVAL; } break; diff --git a/drivers/media/common/uvm/meson_uvm_allocator.h b/drivers/media/common/uvm/meson_uvm_allocator.h index 59423671f..eacd9179e 100644 --- a/drivers/media/common/uvm/meson_uvm_allocator.h +++ b/drivers/media/common/uvm/meson_uvm_allocator.h @@ -81,6 +81,11 @@ struct uvm_fd_data { int commit_display; }; +struct uvm_usage_data { + int fd; + int uvm_data_usage; +}; + struct uvm_meta_data { int fd; int type; @@ -98,6 +103,7 @@ union uvm_ioctl_arg { struct uvm_alloc_data alloc_data; struct uvm_pid_data pid_data; struct uvm_fd_data fd_data; + struct uvm_usage_data usage_data; struct uvm_meta_data meta_data; struct uvm_hook_data hook_data; }; @@ -121,5 +127,10 @@ union uvm_ioctl_arg { struct uvm_hook_data) #define UVM_IOC_DETATCH _IOWR(UVM_IOC_MAGIC, 8, \ struct uvm_hook_data) +#define UVM_IOC_SET_USAGE _IOWR(UVM_IOC_MAGIC, 9, \ + struct uvm_usage_data) +#define UVM_IOC_GET_USAGE _IOWR(UVM_IOC_MAGIC, 10, \ + struct uvm_usage_data) + #endif diff --git a/drivers/media/common/uvm/meson_uvm_core.c b/drivers/media/common/uvm/meson_uvm_core.c index 886f0fbff..43860809f 100644 --- a/drivers/media/common/uvm/meson_uvm_core.c +++ b/drivers/media/common/uvm/meson_uvm_core.c @@ -147,10 +147,14 @@ static void meson_uvm_release(struct dma_buf *dmabuf) if (ua->free) ua->free(ua->obj); - list_for_each_entry_safe(uhmod, uhtmp, &handle->mod_attached, list) + list_for_each_entry_safe(uhmod, uhtmp, &handle->mod_attached, list) { + UVM_PRINTK(1, "%s uhmod:%p uhmod_ref:%u n_attached_mod:%zu\n", + __func__, uhmod, kref_read(&uhmod->ref), handle->n_mod_attached); kref_put(&uhmod->ref, uvm_hook_mod_release); + handle->n_mod_attached--; + } - UVM_PRINTK(1, "%s called, %u\n", __func__, kref_read(&handle->ref)); + UVM_PRINTK(1, "%s handle->ref %u\n", __func__, kref_read(&handle->ref)); kref_put(&handle->ref, uvm_handle_destroy); } @@ -301,7 +305,6 @@ static struct uvm_handle *uvm_handle_alloc(size_t len, size_t align, kref_init(&handle->ref); mutex_init(&handle->lock); - mutex_init(&handle->detachlock); handle->size = len; handle->align = align; handle->flags = flags; @@ -553,43 +556,70 @@ int uvm_attach_hook_mod(struct dma_buf *dmabuf, uhmod->acquire_fence = info->acquire_fence; mutex_lock(&handle->lock); + UVM_PRINTK(1, "attach: type:%d uhmod:%p uhmod->free:%px dmabuf =%p\n", + info->type, uhmod, uhmod->free, dmabuf); list_add_tail(&uhmod->list, &handle->mod_attached); handle->flags &= ~BIT(UVM_DETACH_FLAG); handle->n_mod_attached++; handle->mod_attached_mask |= 1 << (uhmod->type); + UVM_PRINTK(1, "attach: dmabuf =%p uhmod->arg=%p n_attached_mod:%zu\n", + dmabuf, uhmod->arg, handle->n_mod_attached); mutex_unlock(&handle->lock); - UVM_PRINTK(1, "info->type:%d attach ok! dmabuf =%p, handle=%p\n", - info->type, dmabuf, handle); - return 0; } EXPORT_SYMBOL(uvm_attach_hook_mod); -int meson_uvm_getinfo(int shared_fd, int mode_type, char *buf) +int meson_uvm_get_usage(struct dma_buf *dmabuf, size_t *usage) { struct uvm_handle *handle; - struct uvm_hook_mod *uhmod = NULL; - struct dma_buf *dmabuf = NULL; - int ret = 0; - - UVM_PRINTK(1, "uvm_getinfo: shared_fd=%d, mode_type=%d\n", shared_fd, mode_type); - dmabuf = dma_buf_get(shared_fd); if (IS_ERR_OR_NULL(dmabuf)) { - UVM_PRINTK(0, "Invalid dmabuf %s %d\n", __func__, __LINE__); + UVM_PRINTK(0, "invalid dmabuf. %s %d\n", __func__, __LINE__); + return -EINVAL; + } + + if (!dmabuf_is_uvm(dmabuf)) { + UVM_PRINTK(0, "dmabuf is not uvm. %s %d\n", __func__, __LINE__); + return -EINVAL; + } + + handle = dmabuf->priv; + mutex_lock(&handle->lock); + *usage = handle->usage; + mutex_unlock(&handle->lock); + + UVM_PRINTK(1, "%s :%zu dmabuf:%p\n", + __func__, *usage, dmabuf); + return 0; +} +EXPORT_SYMBOL(meson_uvm_get_usage); + +int meson_uvm_getinfo(struct dma_buf *dmabuf, + int mode_type, char *buf) +{ + struct uvm_handle *handle; + struct uvm_hook_mod *uhmod = NULL; + int ret = 0; + + UVM_PRINTK(1, "%s: dmabuf=%p, mode_type=%d\n", + __func__, dmabuf, mode_type); + if (IS_ERR_OR_NULL(dmabuf)) { + UVM_PRINTK(0, "invalid dmabuf. %s %d\n", __func__, __LINE__); return -EINVAL; } if (!dmabuf_is_uvm(dmabuf)) { - dma_buf_put(dmabuf); UVM_PRINTK(0, "dmabuf is not uvm. %s %d\n", __func__, __LINE__); return -EINVAL; } handle = dmabuf->priv; + mutex_lock(&handle->lock); uhmod = uvm_find_hook_mod(handle, mode_type); + mutex_unlock(&handle->lock); + if (uhmod) { ret = uhmod->getinfo(uhmod->arg, buf); dma_buf_put(dmabuf); @@ -618,14 +648,10 @@ static void meson_uvm_core_setinfo(struct uvm_handle *handle, } } -int meson_uvm_setinfo(int shared_fd, int mode_type, char *buf) +int meson_uvm_set_usage(struct dma_buf *dmabuf, size_t usage) { struct uvm_handle *handle; - struct uvm_hook_mod *uhmod = NULL; - struct dma_buf *dmabuf = NULL; - int ret = 0; - dmabuf = dma_buf_get(shared_fd); if (IS_ERR_OR_NULL(dmabuf)) { UVM_PRINTK(0, "invalid dmabuf. %s %d\n", __func__, __LINE__); return -EINVAL; @@ -633,7 +659,33 @@ int meson_uvm_setinfo(int shared_fd, int mode_type, char *buf) if (!dmabuf_is_uvm(dmabuf)) { UVM_PRINTK(0, "dmabuf is not uvm. %s %d\n", __func__, __LINE__); - dma_buf_put(dmabuf); + return -EINVAL; + } + + handle = dmabuf->priv; + mutex_lock(&handle->lock); + handle->usage = usage; + mutex_unlock(&handle->lock); + UVM_PRINTK(1, "%s :%zu dmabuf:%p\n", + __func__, handle->usage, dmabuf); + return 0; +} +EXPORT_SYMBOL(meson_uvm_set_usage); + +int meson_uvm_setinfo(struct dma_buf *dmabuf, + int mode_type, char *buf) +{ + struct uvm_handle *handle; + struct uvm_hook_mod *uhmod = NULL; + int ret = 0; + + if (IS_ERR_OR_NULL(dmabuf)) { + UVM_PRINTK(0, "invalid dmabuf. %s %d\n", __func__, __LINE__); + return -EINVAL; + } + + if (!dmabuf_is_uvm(dmabuf)) { + UVM_PRINTK(0, "dmabuf is not uvm. %s %d\n", __func__, __LINE__); return -EINVAL; } @@ -643,7 +695,9 @@ int meson_uvm_setinfo(int shared_fd, int mode_type, char *buf) dma_buf_put(dmabuf); return 0; } + mutex_lock(&handle->lock); uhmod = uvm_find_hook_mod(handle, mode_type); + mutex_unlock(&handle->lock); if (uhmod) { ret = uhmod->setinfo(uhmod->arg, buf); dma_buf_put(dmabuf); @@ -659,7 +713,7 @@ EXPORT_SYMBOL(meson_uvm_setinfo); int uvm_detach_hook_mod(struct dma_buf *dmabuf, int type) { - UVM_PRINTK(1, "%s called.\n", __func__); + UVM_PRINTK(1, "%s %s called.\n", __func__, current->comm); return uvm_put_hook_mod(dmabuf, type); } EXPORT_SYMBOL(uvm_detach_hook_mod); @@ -670,9 +724,8 @@ static struct uvm_hook_mod *uvm_find_hook_mod(struct uvm_handle *handle, struct uvm_hook_mod *ret = NULL; struct uvm_hook_mod *uhmod = NULL; - UVM_PRINTK(1, "%s called, type-%d.\n", __func__, type); + UVM_PRINTK(1, "%s, type-%d %s called\n", __func__, type, current->comm); - mutex_lock(&handle->lock); if (!list_empty(&handle->mod_attached)) { list_for_each_entry(uhmod, &handle->mod_attached, list) { if (uhmod->type == type) { @@ -681,7 +734,6 @@ static struct uvm_hook_mod *uvm_find_hook_mod(struct uvm_handle *handle, } } } - mutex_unlock(&handle->lock); if (!ret) { UVM_PRINTK(1, "%s fail.\n", __func__); @@ -703,11 +755,18 @@ struct uvm_hook_mod *uvm_get_hook_mod(struct dma_buf *dmabuf, } handle = dmabuf->priv; + mutex_lock(&handle->lock); uhmod = uvm_find_hook_mod(handle, type); - if (uhmod) + if (uhmod) { kref_get(&uhmod->ref); + UVM_PRINTK(1, "%s uhmod:%p, ref:%u type:%d. %s called\n", + __func__, uhmod, kref_read(&uhmod->ref), type, current->comm); + } else { + UVM_PRINTK(1, "%s uhmod is NULL! can not find the match uhmod\n", __func__); + } + mutex_unlock(&handle->lock); - UVM_PRINTK(1, "%s %px, %d.\n", __func__, uhmod, type); + UVM_PRINTK(1, "%s return uhmod:%p dmabuf:%p\n", __func__, uhmod, dmabuf); return uhmod; } EXPORT_SYMBOL(uvm_get_hook_mod); @@ -717,9 +776,15 @@ static void uvm_hook_mod_release(struct kref *kref) struct uvm_hook_mod *uhmod; uhmod = container_of(kref, struct uvm_hook_mod, ref); + + UVM_PRINTK(1, "%s called, entry=%px, uhmod:%px\n", + __func__, &uhmod->list, uhmod); + list_del(&uhmod->list); - UVM_PRINTK(1, "%s called, %px.\n", __func__, uhmod); + + UVM_PRINTK(1, "call uhmod->free:%p start\n", uhmod->free); uhmod->free(uhmod->arg); + UVM_PRINTK(1, "call uhmod->free end and free uhomd\n"); kfree(uhmod); } @@ -729,7 +794,7 @@ int uvm_put_hook_mod(struct dma_buf *dmabuf, int type) struct uvm_hook_mod *uhmod = NULL; int ret = 0; - UVM_PRINTK(1, "%s called, mod_type%d.\n", __func__, type); + UVM_PRINTK(1, "%s, mod_type%d %s called.\n", __func__, type, current->comm); if (IS_ERR_OR_NULL(dmabuf) || !dmabuf_is_uvm(dmabuf)) { UVM_PRINTK(0, "dmabuf is not uvm. %s %d\n", __func__, __LINE__); @@ -738,14 +803,20 @@ int uvm_put_hook_mod(struct dma_buf *dmabuf, int type) handle = dmabuf->priv; - mutex_lock(&handle->detachlock); + mutex_lock(&handle->lock); + uhmod = uvm_find_hook_mod(handle, type); - if (uhmod) + if (uhmod) { + UVM_PRINTK(1, "%s before kref_put uhmod:%p, dmabuf:%p ref:%u\n", + __func__, uhmod, dmabuf, kref_read(&uhmod->ref)); ret = kref_put(&uhmod->ref, uvm_hook_mod_release); - else + } else { + UVM_PRINTK(1, "%s, uhmod is NULL! can not find the match uhmod\n", __func__); ret = -EINVAL; - mutex_unlock(&handle->detachlock); + } + + mutex_unlock(&handle->lock); return ret; } EXPORT_SYMBOL(uvm_put_hook_mod); diff --git a/include/linux/amlogic/meson_uvm_core.h b/include/linux/amlogic/meson_uvm_core.h index ed480e5eb..78c94ae12 100644 --- a/include/linux/amlogic/meson_uvm_core.h +++ b/include/linux/amlogic/meson_uvm_core.h @@ -47,6 +47,7 @@ enum uvm_alloc_flag { * @mod_attached: list of attached module * @n_mod_attached: num of attached module * @mod_attached_mask: mask of attached module + * @usage: buffer usage */ struct uvm_handle { struct kref ref; @@ -68,6 +69,7 @@ struct uvm_handle { struct list_head mod_attached; size_t n_mod_attached; unsigned long mod_attached_mask; + size_t usage; }; /** @@ -131,6 +133,12 @@ enum uvm_hook_mod_type { PROCESS_INVALID }; +enum uvm_data_usage { + UVM_USAGE_VIDEO_PLAY, + UVM_USAGE_IMAGE_PLAY, + UVM_USAGE_INVALID +}; + /** * struct uvm_hook_mod - uvm hook module * @@ -189,11 +197,14 @@ int uvm_attach_hook_mod(struct dma_buf *dmabuf, int uvm_detach_hook_mod(struct dma_buf *dmabuf, int type); -int meson_uvm_getinfo(int shared_fd, +int meson_uvm_getinfo(struct dma_buf *dmabuf, int mode_type, char *buf); -int meson_uvm_setinfo(int shared_fd, +int meson_uvm_setinfo(struct dma_buf *dmabuf, int mode_type, char *buf); +int meson_uvm_get_usage(struct dma_buf *dmabuf, size_t *usage); +int meson_uvm_set_usage(struct dma_buf *dmabuf, size_t usage); + struct uvm_hook_mod *uvm_get_hook_mod(struct dma_buf *dmabuf, int type);