From 5cca30d35db0bf457c1d5205649ea59d93493d51 Mon Sep 17 00:00:00 2001 From: Dennis Cagle Date: Fri, 13 Apr 2018 11:49:03 -0700 Subject: [PATCH 1/2] msm: adsprpc: Use unsigned integer for length values As the length datatype is signed, an attacker can both overflow the calculation or supply a negative number to trick the check into returning an chosen chunk. This can have undesired consequences. Always use unsigned integer types for length values. Change-Id: Ifde2f0d35129014b976507f7723a319c53fabddf Acked-by: Thyagarajan Venkatanarayanan Signed-off-by: Tharun Kumar Merugu Bug: 63165135 CRs-Fixed: 2139538 Signed-off-by: Dennis Cagle (cherry picked from commit c29e11c774b3c59660c1c599b73b7fabf1492d43) Signed-off-by: David Lin Signed-off-by: anupritaisno1 --- drivers/char/adsprpc.c | 92 +++++++++++++++++------------------ drivers/char/adsprpc_compat.c | 14 +++--- drivers/char/adsprpc_shared.h | 25 +++++----- 3 files changed, 66 insertions(+), 65 deletions(-) diff --git a/drivers/char/adsprpc.c b/drivers/char/adsprpc.c index 258c3c9782675..e08f4c6d65f56 100644 --- a/drivers/char/adsprpc.c +++ b/drivers/char/adsprpc.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved. + * Copyright (c) 2012-2015, 2018 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 @@ -80,7 +80,7 @@ static inline uintptr_t buf_page_offset(void *buf) return offset; } -static inline int buf_num_pages(void *buf, ssize_t len) +static inline int buf_num_pages(void *buf, size_t len) { uintptr_t start = buf_page_start(buf) >> PAGE_SHIFT; uintptr_t end = (((uintptr_t) buf + len - 1) & PAGE_MASK) >> PAGE_SHIFT; @@ -152,7 +152,7 @@ struct fastrpc_buf { struct ion_handle *handle; void *virt; ion_phys_addr_t phys; - ssize_t size; + size_t size; int used; }; @@ -239,7 +239,7 @@ struct fastrpc_mmap { ion_phys_addr_t phys; uintptr_t *vaddrin; uintptr_t vaddrout; - ssize_t size; + size_t size; int refs; }; @@ -287,7 +287,7 @@ static int map_iommu_mem(struct ion_handle *handle, struct file_data *fdata, ion_phys_addr_t *iova, unsigned long size) { struct fastrpc_apps *me = &gfa; - struct fastrpc_mmap *map = 0, *mapmatch = 0; + struct fastrpc_mmap *map = NULL, *mapmatch = NULL; struct hlist_node *n; unsigned long len = size; int cid = fdata->cid; @@ -321,7 +321,7 @@ static void unmap_iommu_mem(struct ion_handle *handle, struct file_data *fdata, int cached) { struct fastrpc_apps *me = &gfa; - struct fastrpc_mmap *map = 0, *mapmatch = 0; + struct fastrpc_mmap *map = NULL, *mapmatch = NULL; struct hlist_node *n; int cid = fdata->cid; @@ -356,10 +356,10 @@ static void free_mem(struct fastrpc_buf *buf, struct file_data *fd) } if (!IS_ERR_OR_NULL(buf->virt)) { ion_unmap_kernel(me->iclient, buf->handle); - buf->virt = 0; + buf->virt = NULL; } ion_free(me->iclient, buf->handle); - buf->handle = 0; + buf->handle = NULL; } } @@ -375,11 +375,11 @@ static void free_map(struct fastrpc_mmap *map, struct file_data *fdata) } if (!IS_ERR_OR_NULL(map->virt)) { ion_unmap_kernel(me->iclient, map->handle); - map->virt = 0; + map->virt = NULL; } ion_free(me->iclient, map->handle); } - map->handle = 0; + map->handle = NULL; } static int alloc_mem(struct fastrpc_buf *buf, struct file_data *fdata) @@ -390,8 +390,8 @@ static int alloc_mem(struct fastrpc_buf *buf, struct file_data *fdata) int err = 0; int cid = fdata->cid; unsigned int heap; - buf->handle = 0; - buf->virt = 0; + buf->handle = NULL; + buf->virt = NULL; buf->phys = 0; heap = me->channel[cid].smmu.enabled ? ION_HEAP(ION_IOMMU_HEAP_ID) : ION_HEAP(ION_ADSP_HEAP_ID); @@ -426,7 +426,7 @@ static int context_restore_interrupted(struct fastrpc_apps *me, struct smq_invoke_ctx **po) { int err = 0; - struct smq_invoke_ctx *ctx = 0, *ictx = 0; + struct smq_invoke_ctx *ctx = NULL, *ictx = NULL; struct hlist_node *n; struct fastrpc_ioctl_invoke *invoke = &invokefd->inv; spin_lock(&me->clst.hlock); @@ -522,7 +522,7 @@ static int context_alloc(struct fastrpc_apps *me, uint32_t kernel, struct smq_invoke_ctx **po) { int err = 0, bufs, ii, size = 0; - struct smq_invoke_ctx *ctx = 0; + struct smq_invoke_ctx *ctx = NULL; struct smq_context_list *clst = &me->clst; struct fastrpc_ioctl_invoke *invoke = &invokefd->inv; @@ -536,7 +536,7 @@ static int context_alloc(struct fastrpc_apps *me, uint32_t kernel, bufs * sizeof(*ctx->handles); } - VERIFY(err, 0 != (ctx = kzalloc(sizeof(*ctx) + size, GFP_KERNEL))); + VERIFY(err, NULL != (ctx = kzalloc(sizeof(*ctx) + size, GFP_KERNEL))); if (err) goto bail; @@ -545,8 +545,8 @@ static int context_alloc(struct fastrpc_apps *me, uint32_t kernel, ctx->apps = me; ctx->fdata = fdata; ctx->pra = (remote_arg_t *)(&ctx[1]); - ctx->fds = invokefd->fds == 0 ? 0 : (int *)(&ctx->pra[bufs]); - ctx->handles = invokefd->fds == 0 ? 0 : + ctx->fds = invokefd->fds == NULL ? NULL : (int *)(&ctx->pra[bufs]); + ctx->handles = invokefd->fds == NULL ? NULL : (struct ion_handle **)(&ctx->fds[bufs]); if (!kernel) { VERIFY(err, 0 == copy_from_user(ctx->pra, invoke->pra, @@ -681,7 +681,7 @@ static void context_notify_user(struct smq_invoke_ctx *ctx, int retval) static void context_notify_all_users(struct smq_context_list *me, int cid) { - struct smq_invoke_ctx *ictx = 0; + struct smq_invoke_ctx *ictx = NULL; struct hlist_node *n; spin_lock(&me->hlock); hlist_for_each_entry_safe(ictx, n, &me->pending, hn) { @@ -706,10 +706,10 @@ static void context_list_ctor(struct smq_context_list *me) static void context_list_dtor(struct fastrpc_apps *me, struct smq_context_list *clst) { - struct smq_invoke_ctx *ictx = 0, *ctxfree; + struct smq_invoke_ctx *ictx = NULL, *ctxfree; struct hlist_node *n; do { - ctxfree = 0; + ctxfree = NULL; spin_lock(&clst->hlock); hlist_for_each_entry_safe(ictx, n, &clst->interrupted, hn) { hlist_del(&ictx->hn); @@ -742,7 +742,7 @@ static int get_page_list(uint32_t kernel, struct smq_invoke_ctx *ctx) struct fastrpc_buf *ibuf = &ctx->dev->buf; struct fastrpc_buf *obuf = &ctx->obuf; remote_arg_t *pra = ctx->pra; - ssize_t rlen; + size_t rlen; uint32_t sc = ctx->sc; int cid = ctx->fdata->cid; int i, err = 0; @@ -769,7 +769,7 @@ static int get_page_list(uint32_t kernel, struct smq_invoke_ctx *ctx) for (i = 0; i < inbufs + outbufs; ++i) { void *buf; int num; - ssize_t len; + size_t len; list[i].num = 0; list[i].pgidx = 0; @@ -876,14 +876,14 @@ static int get_args(uint32_t kernel, struct smq_invoke_ctx *ctx, { struct fastrpc_apps *me = &gfa; struct smq_invoke_buf *list; - struct fastrpc_buf *pbuf = &ctx->obuf, *obufs = 0; + struct fastrpc_buf *pbuf = &ctx->obuf, *obufs = NULL; struct smq_phy_page *pages; struct vm_area_struct *vma; struct ion_handle **handles = ctx->handles; void *args; remote_arg_t *pra = ctx->pra; remote_arg_t *rpra = ctx->rpra; - ssize_t rlen, used, size, copylen = 0; + size_t rlen, used, size, copylen = 0; uint32_t sc = ctx->sc, start; int i, inh, bufs = 0, err = 0, oix; int inbufs = REMOTE_SCALARS_INBUFS(sc); @@ -976,7 +976,7 @@ static int get_args(uint32_t kernel, struct smq_invoke_ctx *ctx, /* copy non ion buffers */ for (oix = 0; oix < inbufs + outbufs; ++oix) { int i = ctx->overps[oix]->raix; - ssize_t mlen = ctx->overps[oix]->mend - ctx->overps[oix]->mstart; + size_t mlen = ctx->overps[oix]->mend - ctx->overps[oix]->mstart; if (!pra[i].buf.len) continue; if (list[i].num) @@ -1271,7 +1271,7 @@ static void free_dev(struct fastrpc_device *dev, struct file_data *fdata) static int alloc_dev(struct fastrpc_device **dev, struct file_data *fdata) { int err = 0; - struct fastrpc_device *fd = 0; + struct fastrpc_device *fd = NULL; VERIFY(err, 0 != try_module_get(THIS_MODULE)); if (err) @@ -1299,7 +1299,7 @@ static int get_dev(struct fastrpc_apps *me, struct file_data *fdata, struct fastrpc_device **rdev) { struct hlist_head *head; - struct fastrpc_device *dev = 0, *devfree = 0; + struct fastrpc_device *dev = NULL, *devfree = NULL; struct hlist_node *n; uint32_t h = hash_32(current->tgid, RPC_HASH_BITS); int err = 0; @@ -1345,7 +1345,7 @@ static int fastrpc_internal_invoke(struct fastrpc_apps *me, uint32_t mode, struct fastrpc_ioctl_invoke_fd *invokefd, struct file_data *fdata) { - struct smq_invoke_ctx *ctx = 0; + struct smq_invoke_ctx *ctx = NULL; struct fastrpc_ioctl_invoke *invoke = &invokefd->inv; int cid = fdata->cid; int interrupted = 0; @@ -1434,8 +1434,8 @@ static int fastrpc_init_process(struct file_data *fdata, { int err = 0; struct fastrpc_ioctl_invoke_fd ioctl; - struct smq_phy_page *pages = 0; - struct fastrpc_mmap *map = 0; + struct smq_phy_page *pages = NULL; + struct fastrpc_mmap *map = NULL; int npages = 0; struct fastrpc_apps *me = &gfa; if (init->flags == FASTRPC_INIT_ATTACH) { @@ -1590,7 +1590,7 @@ static int fastrpc_munmap_on_dsp(struct fastrpc_apps *me, struct { int pid; uintptr_t vaddrout; - ssize_t size; + size_t size; } inargs; inargs.pid = current->tgid; @@ -1644,9 +1644,9 @@ static int map_buffer(struct fastrpc_apps *me, struct file_data *fdata, struct smq_phy_page **ppages, int *pnpages) { struct ion_client *clnt = gfa.iclient; - struct ion_handle *handle = 0; - struct fastrpc_mmap *map = 0, *mapmatch = 0; - struct smq_phy_page *pages = 0; + struct ion_handle *handle = NULL; + struct fastrpc_mmap *map = NULL, *mapmatch = NULL; + struct smq_phy_page *pages = NULL; struct hlist_node *n; uintptr_t vaddrout = 0; int num; @@ -1723,8 +1723,8 @@ static int fastrpc_internal_mmap(struct fastrpc_apps *me, struct fastrpc_ioctl_mmap *mmap) { - struct fastrpc_mmap *map = 0; - struct smq_phy_page *pages = 0; + struct fastrpc_mmap *map = NULL; + struct smq_phy_page *pages = NULL; int num = 0; int err = 0; VERIFY(err, 0 == map_buffer(me, fdata, mmap->fd, (char *)mmap->vaddrin, @@ -1754,7 +1754,7 @@ static void cleanup_current_dev(struct file_data *fdata) struct fastrpc_device *dev, *devfree; rnext: - devfree = dev = 0; + devfree = dev = NULL; spin_lock(&me->hlock); head = &me->htbl[h]; hlist_for_each_entry_safe(dev, n, head, hn) { @@ -1792,9 +1792,9 @@ static int fastrpc_device_release(struct inode *inode, struct file *file) struct file_data *fdata = (struct file_data *)file->private_data; struct fastrpc_apps *me = &gfa; struct smq_context_list *clst = &me->clst; - struct smq_invoke_ctx *ictx = 0, *ctxfree; + struct smq_invoke_ctx *ictx = NULL, *ctxfree; struct hlist_node *n; - struct fastrpc_mmap *map = 0; + struct fastrpc_mmap *map = NULL; int cid = MINOR(inode->i_rdev); if (!fdata) @@ -1802,7 +1802,7 @@ static int fastrpc_device_release(struct inode *inode, struct file *file) (void)fastrpc_release_current_dsp_process(fdata); do { - ctxfree = 0; + ctxfree = NULL; spin_lock(&clst->hlock); hlist_for_each_entry_safe(ictx, n, &clst->interrupted, hn) { if ((ictx->tgid == current->tgid) && @@ -1840,7 +1840,7 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) mutex_lock(&me->smd_mutex); ssrcount = me->channel[cid].ssrcount; if ((kref_get_unless_zero(&me->channel[cid].kref) == 0) || - (me->channel[cid].chan == 0)) { + (me->channel[cid].chan == NULL)) { VERIFY(err, 0 == smd_named_open_on_edge( FASTRPC_SMD_GUID, gcinfo[cid].channel, @@ -1860,9 +1860,9 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) } mutex_unlock(&me->smd_mutex); - filp->private_data = 0; + filp->private_data = NULL; if (0 != try_module_get(THIS_MODULE)) { - struct file_data *fdata = 0; + struct file_data *fdata = NULL; /* This call will cause a dev to be created * which will addref this module */ @@ -1892,7 +1892,7 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) completion_bail: smd_close(me->channel[cid].chan); - me->channel[cid].chan = 0; + me->channel[cid].chan = NULL; smd_bail: mutex_unlock(&me->smd_mutex); return err; @@ -1914,7 +1914,7 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int ioctl_num, switch (ioctl_num) { case FASTRPC_IOCTL_INVOKE_FD: case FASTRPC_IOCTL_INVOKE: - invokefd.fds = 0; + invokefd.fds = NULL; size = (ioctl_num == FASTRPC_IOCTL_INVOKE) ? sizeof(invokefd.inv) : sizeof(invokefd); VERIFY(err, 0 == copy_from_user(&invokefd, param, size)); @@ -1992,7 +1992,7 @@ static int fastrpc_restart_notifier_cb(struct notifier_block *nb, ctx->ssrcount++; if (ctx->chan) { smd_close(ctx->chan); - ctx->chan = 0; + ctx->chan = NULL; pr_info("'closed /dev/%s c %d %d'\n", gcinfo[cid].name, MAJOR(me->dev_no), cid); } diff --git a/drivers/char/adsprpc_compat.c b/drivers/char/adsprpc_compat.c index ee324dcf91f0e..160ad1851c9a8 100644 --- a/drivers/char/adsprpc_compat.c +++ b/drivers/char/adsprpc_compat.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014 The Linux Foundation. All rights reserved. + * Copyright (c) 2014,2017 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 @@ -33,7 +33,7 @@ struct compat_remote_buf { compat_uptr_t pv; /* buffer pointer */ - compat_ssize_t len; /* length of buffer */ + compat_size_t len; /* length of buffer */ }; union compat_remote_arg { @@ -56,13 +56,13 @@ struct compat_fastrpc_ioctl_mmap { compat_int_t fd; /* ion fd */ compat_uint_t flags; /* flags for dsp to map with */ compat_uptr_t vaddrin; /* optional virtual address */ - compat_ssize_t size; /* size */ + compat_size_t size; /* size */ compat_uptr_t vaddrout; /* dsps virtual address */ }; struct compat_fastrpc_ioctl_munmap { compat_uptr_t vaddrout; /* address to unmap */ - compat_ssize_t size; /* size */ + compat_size_t size; /* size */ }; struct compat_fastrpc_ioctl_init { @@ -81,7 +81,7 @@ static int compat_get_fastrpc_ioctl_invoke( unsigned int cmd) { compat_uint_t u, sc; - compat_ssize_t s; + compat_size_t s; compat_uptr_t p; struct fastrpc_ioctl_invoke_fd *inv; union compat_remote_arg *pra32; @@ -164,7 +164,7 @@ static int compat_get_fastrpc_ioctl_mmap( { compat_uint_t u; compat_int_t i; - compat_ssize_t s; + compat_size_t s; compat_uptr_t p; int err; @@ -198,7 +198,7 @@ static int compat_get_fastrpc_ioctl_munmap( struct fastrpc_ioctl_munmap __user *unmap) { compat_uptr_t p; - compat_ssize_t s; + compat_size_t s; int err; err = get_user(p, &unmap32->vaddrout); diff --git a/drivers/char/adsprpc_shared.h b/drivers/char/adsprpc_shared.h index c8e31369cef16..b8e5f23a56c04 100644 --- a/drivers/char/adsprpc_shared.h +++ b/drivers/char/adsprpc_shared.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved. + * Copyright (c) 2012-2014,2017 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 @@ -93,7 +93,7 @@ do {\ struct remote_buf { void *pv; /* buffer pointer */ - ssize_t len; /* length of buffer */ + size_t len; /* length of buffer */ }; union remote_arg { @@ -114,25 +114,25 @@ struct fastrpc_ioctl_invoke_fd { struct fastrpc_ioctl_init { uint32_t flags; /* one of FASTRPC_INIT_* macros */ - uintptr_t __user file; /* pointer to elf file */ - int32_t filelen; /* elf file length */ + uintptr_t file; /* pointer to elf file */ + uint32_t filelen; /* elf file length */ int32_t filefd; /* ION fd for the file */ - uintptr_t __user mem; /* mem for the PD */ - int32_t memlen; /* mem length */ + uintptr_t mem; /* mem for the PD */ + uint32_t memlen; /* mem length */ int32_t memfd; /* ION fd for the mem */ }; struct fastrpc_ioctl_munmap { uintptr_t vaddrout; /* address to unmap */ - ssize_t size; /* size */ + size_t size; /* size */ }; struct fastrpc_ioctl_mmap { int fd; /* ion fd */ uint32_t flags; /* flags for dsp to map with */ - uintptr_t __user *vaddrin; /* optional virtual address */ - ssize_t size; /* size */ + uintptr_t vaddrin; /* optional virtual address */ + size_t size; /* size */ uintptr_t vaddrout; /* dsps virtual address */ }; @@ -144,7 +144,7 @@ struct smq_null_invoke { struct smq_phy_page { unsigned long addr; /* physical address */ - ssize_t size; /* size of contiguous region */ + size_t size; /* size of contiguous region */ }; struct smq_invoke_buf { @@ -171,14 +171,15 @@ struct smq_invoke_rsp { static inline struct smq_invoke_buf *smq_invoke_buf_start(remote_arg_t *pra, uint32_t sc) { - int len = REMOTE_SCALARS_LENGTH(sc); + unsigned int len = REMOTE_SCALARS_LENGTH(sc); + return (struct smq_invoke_buf *)(&pra[len]); } static inline struct smq_phy_page *smq_phy_page_start(uint32_t sc, struct smq_invoke_buf *buf) { - int nTotal = REMOTE_SCALARS_INBUFS(sc) + REMOTE_SCALARS_OUTBUFS(sc); + uint32_t nTotal = REMOTE_SCALARS_INBUFS(sc)+REMOTE_SCALARS_OUTBUFS(sc); return (struct smq_phy_page *)(&buf[nTotal]); } From 0d24559b9917347d42f4e2a901700fdec1d9a38d Mon Sep 17 00:00:00 2001 From: Banajit Goswami Date: Mon, 10 Apr 2017 19:56:25 -0700 Subject: [PATCH 2/2] soc: q6dspv2: apr: fix client registration refcount Audio Packet Router (APR) is used by multiple audio services to communicate between APSS and ADSP. These audio services registers for service level APR communication (port 0xFFFFFFFF), or for session level APR communication (using port 0x101 etc.). The services might choose to call apr_register for any port at random. The expectation is that the refcounting for the number of ports registered with APR for any specific service, is handled irrespective of the order in which registrations are done. The current logic fails to handle the refcounting when apr_register is called for 0xFFFFFFFF before other session based ports. Fix this correctly using the service count (svc_cnt) variable in apr_svc. CRs-fixed: 2022490 Bug: 34088848 Change-Id: I2fcd1269facf24d509db0d90314e0d2545a2ad67 Signed-off-by: Banajit Goswami Signed-off-by: anupritaisno1 # Conflicts: # drivers/soc/qcom/qdsp6v2/apr.c --- drivers/soc/qcom/qdsp6v2/apr.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/drivers/soc/qcom/qdsp6v2/apr.c b/drivers/soc/qcom/qdsp6v2/apr.c index cb4cda4f7f313..7c945dff8daf0 100644 --- a/drivers/soc/qcom/qdsp6v2/apr.c +++ b/drivers/soc/qcom/qdsp6v2/apr.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2010-2015, 2016, The Linux Foundation. All rights reserved. +/* Copyright (c) 2010-2015, 2016, 2018, 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 @@ -404,19 +404,19 @@ struct apr_svc *apr_register(char *dest, char *svc_name, apr_fn svc_fn, mutex_unlock(&svc->m_lock); return NULL; } - if (!svc->port_cnt && !svc->svc_cnt) + if (!svc->svc_cnt) clnt->svc_cnt++; svc->port_cnt++; svc->port_fn[temp_port] = svc_fn; svc->port_priv[temp_port] = priv; + svc->svc_cnt++; } else { if (!svc->fn) { - if (!svc->port_cnt && !svc->svc_cnt) + if (!svc->svc_cnt) clnt->svc_cnt++; svc->fn = svc_fn; - if (svc->port_cnt) - svc->svc_cnt++; svc->priv = priv; + svc->svc_cnt++; } } @@ -622,24 +622,17 @@ int apr_deregister(void *handle) client_id = svc->client_id; clnt = &client[dest_id][client_id]; - if (svc->port_cnt > 0 || svc->svc_cnt > 0) { + if (svc->svc_cnt > 0) { if (svc->port_cnt) svc->port_cnt--; - else if (svc->svc_cnt) - svc->svc_cnt--; - if (!svc->port_cnt && !svc->svc_cnt) { + svc->svc_cnt--; + if (!svc->svc_cnt) { client[dest_id][client_id].svc_cnt--; - svc->need_reset = 0x0; - } - } else if (client[dest_id][client_id].svc_cnt > 0) { - client[dest_id][client_id].svc_cnt--; - if (!client[dest_id][client_id].svc_cnt) { - svc->need_reset = 0x0; pr_debug("%s: service is reset %pK\n", __func__, svc); } } - if (!svc->port_cnt && !svc->svc_cnt) { + if (!svc->svc_cnt) { svc->priv = NULL; svc->id = 0; svc->fn = NULL;