From 39bab9b84517e757e407fd76879973f7f9b5855f Mon Sep 17 00:00:00 2001 From: Mohammed Nayeem Ur Rahman Date: Mon, 20 Apr 2020 15:19:38 +0530 Subject: [PATCH] msm: ADSPRPC: Fix to avoid race condition and use after free Current mmap and munmap use same mutex but munmap_fd does not use the same. This can introduce race condition between mmap and munmap_fd. Also there is a use after free scenario in get_args and init_process as only mmap_create is protected and the map can be freed after this. Unifying mutex to avoid race condition. Restricting munmap_fd only for persist bufs to avoid this. Change-Id: I2adf631b1e61c2274a14a3645a5e555f5d248645 Acked-by: Ekansh Gupta Signed-off-by: Mohammed Nayeem Ur Rahman --- drivers/char/adsprpc.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/char/adsprpc.c b/drivers/char/adsprpc.c index 0f8abacb9998..7d53989edbe5 100644 --- a/drivers/char/adsprpc.c +++ b/drivers/char/adsprpc.c @@ -4168,6 +4168,11 @@ bail: return err; } +/* + * fastrpc_internal_munmap_fd can only be used for buffers + * mapped with persist attributes. This can only be called + * once for any persist buffer + */ static int fastrpc_internal_munmap_fd(struct fastrpc_file *fl, struct fastrpc_ioctl_munmap_fd *ud) { @@ -4177,7 +4182,7 @@ static int fastrpc_internal_munmap_fd(struct fastrpc_file *fl, VERIFY(err, (fl && ud)); if (err) { err = -EINVAL; - goto bail; + return err; } VERIFY(err, fl->dsp_proc_init == 1); if (err) { @@ -4185,8 +4190,9 @@ static int fastrpc_internal_munmap_fd(struct fastrpc_file *fl, "user application %s trying to unmap without initialization\n", current->comm); err = -EHOSTDOWN; - goto bail; + return err; } + mutex_lock(&fl->internal_map_mutex); mutex_lock(&fl->map_mutex); err = fastrpc_mmap_find(fl, ud->fd, ud->va, ud->len, 0, 0, &map); if (err) { @@ -4197,10 +4203,13 @@ static int fastrpc_internal_munmap_fd(struct fastrpc_file *fl, mutex_unlock(&fl->map_mutex); goto bail; } - if (map) + if (map && (map->attr & FASTRPC_ATTR_KEEP_MAP)) { + map->attr = map->attr & (~FASTRPC_ATTR_KEEP_MAP); fastrpc_mmap_free(map, 0); + } mutex_unlock(&fl->map_mutex); bail: + mutex_unlock(&fl->internal_map_mutex); return err; } @@ -4320,7 +4329,7 @@ static int fastrpc_internal_mmap(struct fastrpc_file *fl, "user application %s trying to map without initialization\n", current->comm); err = -EHOSTDOWN; - goto bail; + return err; } mutex_lock(&fl->internal_map_mutex); if ((ud->flags == ADSP_MMAP_ADD_PAGES) ||