From c6e7698c0cf35551ab16d16ac5e21e2272644734 Mon Sep 17 00:00:00 2001 From: Santosh Sakore Date: Tue, 20 Aug 2024 12:31:31 +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: Om Deore Change-Id: I19ae21230bf11fe89858b10c9069a5daccabc392 Signed-off-by: Santosh Sakore --- 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 236a22608547..c13641940079 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);