This fixes the incredibly broken behavior of fastrpc_context_free(),
which calls fastrpc_map_put() but does not remove the map from the list
when it is free'd, causing use-after-free errors.

fl->lock needs to be held not just for list_del(), but also kref_put, to
avoid a race condition with fastrpc_map_find() logic.

Signed-off-by: Jonathan Marek <jonat...@marek.ca>
---
 drivers/misc/fastrpc.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 4fabea0c1551..170352b43ab6 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -245,19 +245,21 @@ static void fastrpc_free_map(struct kref *ref)
                dma_buf_put(map->buf);
        }
 
+       list_del(&map->node);
+
        kfree(map);
 }
 
 static void fastrpc_map_put(struct fastrpc_map *map)
 {
-       if (map)
-               kref_put(&map->refcount, fastrpc_free_map);
+       spin_lock(&map->fl->lock);
+       kref_put(&map->refcount, fastrpc_free_map);
+       spin_unlock(&map->fl->lock);
 }
 
 static void fastrpc_map_get(struct fastrpc_map *map)
 {
-       if (map)
-               kref_get(&map->refcount);
+       kref_get(&map->refcount);
 }
 
 static int fastrpc_map_find(struct fastrpc_user *fl, int fd,
@@ -351,8 +353,10 @@ static void fastrpc_context_free(struct kref *ref)
        ctx = container_of(ref, struct fastrpc_invoke_ctx, refcount);
        cctx = ctx->cctx;
 
-       for (i = 0; i < ctx->nscalars; i++)
-               fastrpc_map_put(ctx->maps[i]);
+       for (i = 0; i < ctx->nscalars; i++) {
+               if (ctx->maps[i])
+                       fastrpc_map_put(ctx->maps[i]);
+       }
 
        if (ctx->buf)
                fastrpc_buf_free(ctx->buf);
@@ -1103,12 +1107,8 @@ static int fastrpc_init_create_process(struct 
fastrpc_user *fl,
        fl->init_mem = NULL;
        fastrpc_buf_free(imem);
 err_alloc:
-       if (map) {
-               spin_lock(&fl->lock);
-               list_del(&map->node);
-               spin_unlock(&fl->lock);
+       if (map)
                fastrpc_map_put(map);
-       }
 err:
        kfree(args);
 
@@ -1185,10 +1185,9 @@ static int fastrpc_device_release(struct inode *inode, 
struct file *file)
                fastrpc_context_put(ctx);
        }
 
-       list_for_each_entry_safe(map, m, &fl->maps, node) {
-               list_del(&map->node);
+       list_for_each_entry_safe(map, m, &fl->maps, node)
                fastrpc_map_put(map);
-       }
+       WARN_ON(!list_empty(&fl->maps));
 
        list_for_each_entry_safe(buf, b, &fl->mmaps, node) {
                list_del(&buf->node);
-- 
2.26.1

Reply via email to