From a63ba862ae29aee603fc93b30f32320c3e8f61a0 Mon Sep 17 00:00:00 2001 From: Sipra Patel Date: Thu, 19 Sep 2024 11:10:57 +0530 Subject: [PATCH] adsprpc: Handle UAF scenario in put_args Currently, the DSP updates header buffers with unused DMA handle fds. In the put_args section, if any DMA handle FDs are present in the header buffer, the corresponding map is freed. However, since the header buffer is exposed to users in unsigned PD, users can update invalid FDs. If this invalid FD matches with any FD that is already in use, it could lead to a use-after-free (UAF) vulnerability. As a solution,add DMA handle references for DMA FDs, and the map for the FD will be freed only when a reference is found. Acked-by: quic_anane Change-Id: I83ae12329003db14dae8d430ab74925f2a30947b Signed-off-by: Ansa Ahmed --- Mot-CRs-fixed: (CR) CVE- fixed: CVE-2024-21455 CRs Fixed: QC-CR#3875202 Signed-off-by: Sipra Patel Reviewed-on: https://gerrit.mot.com/3075244 SME-Granted: SME Approvals Granted SLTApproved: Slta Waiver Tested-by: Jira Key Reviewed-by: Varun Shrivastava Reviewed-by: Xiangpo Zhao Submit-Approved: Jira Key --- drivers/char/adsprpc.c | 71 +++++++++++++++++++++++++++++++----------- 1 file changed, 53 insertions(+), 18 deletions(-) diff --git a/drivers/char/adsprpc.c b/drivers/char/adsprpc.c index 5debb2b4bc9c..4d97d3811f8a 100644 --- a/drivers/char/adsprpc.c +++ b/drivers/char/adsprpc.c @@ -584,6 +584,8 @@ struct fastrpc_mmap { struct timespec64 map_end_time; bool is_filemap; /* flag to indicate map used in process init */ unsigned int ctx_refs; /* Indicates reference count for context map */ + /* Map in use for dma handle */ + unsigned int dma_handle_refs; }; enum fastrpc_perfkeys { @@ -1213,9 +1215,14 @@ static int fastrpc_mmap_remove(struct fastrpc_file *fl, int fd, uintptr_t va, return 0; } hlist_for_each_entry_safe(map, n, &fl->maps, hn) { - /* Remove if only one reference map and no context map */ - if (map->refs == 1 && !map->ctx_refs && - map->raddr == va && map->raddr + map->len == va + len && + if ((fd < 0 || map->fd == fd) && + map->raddr == va && + map->raddr + map->len == va + len && + /* Remove if only one reference map and no context map */ + map->refs == 1 && + !map->ctx_refs && + /* Remove map only if it isn't being used by DSP */ + !map->dma_handle_refs && /* Remove map if not used in process initialization */ !map->is_filemap) { match = map; @@ -1254,8 +1261,9 @@ static void fastrpc_mmap_free(struct fastrpc_mmap *map, uint32_t flags) if (map->flags == ADSP_MMAP_HEAP_ADDR || map->flags == ADSP_MMAP_REMOTE_HEAP_ADDR) { spin_lock(&me->hlock); - map->refs--; - if (!map->refs && !map->is_persistent && !map->ctx_refs) + if (map->refs) + map->refs--; + if (!map->refs && !map->is_persistent) hlist_del_init(&map->hn); spin_unlock(&me->hlock); if (map->refs > 0) { @@ -1270,8 +1278,13 @@ static void fastrpc_mmap_free(struct fastrpc_mmap *map, uint32_t flags) spin_unlock(&me->hlock); } } else { - map->refs--; - if (!map->refs && !map->ctx_refs) + if (map->refs) + map->refs--; + /* flags is passed as 1 during fastrpc_file_free + * (ie process exit), so that maps will be cleared + * even though references are present. + */ + if (!map->refs && !map->ctx_refs && !map->dma_handle_refs) hlist_del_init(&map->hn); if (map->refs > 0 && !flags) return; @@ -2492,12 +2505,13 @@ static int get_args(uint32_t kernel, struct smq_invoke_ctx *ctx) FASTRPC_ATTR_NOVA, 0, 0, dmaflags, &ctx->maps[i]); if (!err && ctx->maps[i]) - ctx->maps[i]->ctx_refs++; + ctx->maps[i]->dma_handle_refs++; if (err) { for (j = bufs; j < i; j++) { - if (ctx->maps[j] && ctx->maps[j]->ctx_refs) - ctx->maps[j]->ctx_refs--; - fastrpc_mmap_free(ctx->maps[j], 0); + if (ctx->maps[j] && ctx->maps[j]->dma_handle_refs) { + ctx->maps[j]->dma_handle_refs--; + fastrpc_mmap_free(ctx->maps[j], 0); + } } mutex_unlock(&ctx->fl->map_mutex); goto bail; @@ -2635,13 +2649,33 @@ static int get_args(uint32_t kernel, struct smq_invoke_ctx *ctx) rpra[i].buf.pv = buf; } PERF_END); + /* Since we are not holidng map_mutex during get args whole time + * it is possible that dma handle map may be removed by some invalid + * fd passed by DSP. Inside the lock check if the map present or not + */ + mutex_lock(&ctx->fl->map_mutex); for (i = bufs; i < bufs + handles; ++i) { - struct fastrpc_mmap *map = ctx->maps[i]; - if (map) { - pages[i].addr = map->phys; - pages[i].size = map->size; + struct fastrpc_mmap *mmap = NULL; + /* check if map was created */ + if (ctx->maps[i]) { + /* check if map still exist */ + if (!fastrpc_mmap_find(ctx->fl, ctx->fds[i], 0, 0, + 0, 0, &mmap)) { + if (mmap) { + pages[i].addr = mmap->phys; + pages[i].size = mmap->size; + } + + } else { + /* map already freed by some other call */ + mutex_unlock(&ctx->fl->map_mutex); + ADSPRPC_ERR("could not find map associated with dma handle fd %d\n", + ctx->fds[i]); + goto bail; + } } } + mutex_unlock(&ctx->fl->map_mutex); fdlist = (uint64_t *)&pages[bufs + handles]; crclist = (uint32_t *)&fdlist[M_FDLIST]; /* reset fds, crc and early wakeup hint memory */ @@ -2842,9 +2876,10 @@ static int put_args(uint32_t kernel, struct smq_invoke_ctx *ctx, break; if (!fastrpc_mmap_find(ctx->fl, (int)fdlist[i], 0, 0, 0, 0, &mmap)) { - if (mmap && mmap->ctx_refs) - mmap->ctx_refs--; - fastrpc_mmap_free(mmap, 0); + if (mmap && mmap->dma_handle_refs) { + mmap->dma_handle_refs = 0; + fastrpc_mmap_free(mmap, 0); + } } } mutex_unlock(&ctx->fl->map_mutex);