fastrpc: fixes for 7.2

From: Srinivas Kandagatla <[email protected]>

Hi Greg,

This series collects a handful of fastrpc fixes that have accumulated on the list. All patches are small, target long-standing issues, and are tagged for stable.

The fixes fall into two buckets:

Memory leaks:
- Audio PD memory pool never registered its initial buffer because pageslen was left at zero, so the pool was always empty and every allocation fell back to the remote heap (patch 1).
- fastrpc_device_open() takes a channel ctx reference before allocating a session; the -EBUSY path on session-alloc failure never dropped it (patch 4).
- fastrpc_channel_ctx_free() never destroyed ctx_idr, leaking the IDR backing storage on channel teardown (patch 5).

Locking / list corruption:
- fastrpc_req_munmap() removed the buffer from fl->mmaps only after the DSP unmap returned, allowing two concurrent unmaps to race on the same entry. Detach the buffer under fl->lock first and re-add it if the DSP call fails (patch 2).
- The -ERESTARTSYS path in fastrpc_internal_invoke() walked fl->mmaps and spliced it onto cctx->invoke_interrupted_mmaps without holding fl->lock, racing with every other mmaps accessor (patch 3).

Please queue for 7.2.

Thanks,
Srini

Anandu Krishnan E (1):
misc: fastrpc: fix channel ctx ref leak when session alloc fails

Eddie Lin (1):
misc: fastrpc: fix memory leak in fastrpc_channel_ctx_free

Ekansh Gupta (2):
misc: fastrpc: Fix initial memory allocation for Audio PD memory pool
misc: fastrpc: Remove buffer from list prior to unmap operation

Junrui Luo (1):
misc: fastrpc: take fl->lock when moving mmaps on interrupted invoke

 drivers/misc/fastrpc.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

--
2.53.0

From: Ekansh Gupta <[email protected]>

The initial buffer allocated for the Audio PD memory pool is never added to the pool because pageslen is set to 0. As a result, the buffer is not registered with Audio PD and is never used, causing a memory leak. Audio PD immediately falls back to allocating memory from the remote heap since the pool starts out empty.

Fix this by setting pageslen to 1 so that the initially allocated buffer is correctly registered and becomes part of the Audio PD memory pool.

---
 drivers/misc/fastrpc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

index f3a49384586d..4f01ebfa6f95 100644
@@ -1370,7 +1370,9 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 		err = PTR_ERR(name);
 		goto err;
 	}
-
+	inbuf.client_id = fl->client_id;
+	inbuf.namelen = init.namelen;
+	inbuf.pageslen = 0;
 	if (!fl->cctx->remote_heap) {
 		err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
 						&fl->cctx->remote_heap);

@@ -1393,12 +1395,10 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 				goto err_map;
 			}
 			scm_done = true;
+			inbuf.pageslen = 1;
 		}
 	}
 
-	inbuf.client_id = fl->client_id;
-	inbuf.namelen = init.namelen;
-	inbuf.pageslen = 0;
 	fl->pd = USER_PD;
 
 	args[0].ptr = (u64)(uintptr_t)&inbuf;

2.53.0

From: Ekansh Gupta <[email protected]>

fastrpc_req_munmap_impl() is called to unmap any buffer. The buffer is getting removed from the list after it is unmapped from DSP. This can create potential race conditions if multiple threads invoke unmap concurrently, where one thread may remove the entry from the list while another thread's unmap operation is still ongoing.

Fix this by removing the buffer entry from the list before calling the unmap operation. If the unmap fails, the entry is re-added to the list so that userspace can retry the unmap, or alternatively, the buffer will be cleaned up during device release when the DSP process is torn down and all DSP-side mappings are freed along with remaining buffers in the list.

---
 drivers/misc/fastrpc.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

index 4f01ebfa6f95..12dcd2e737c3 100644
@@ -1889,9 +1889,6 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf *
 				      &args[0]);
 	if (!err) {
 		dev_dbg(dev, "unmmap\tpt 0x%09lx OK\n", buf->raddr);
-		spin_lock(&fl->lock);
-		list_del(&buf->node);
-		spin_unlock(&fl->lock);
 		fastrpc_buf_free(buf);
 	} else {
 		dev_err(dev, "unmmap\tpt 0x%09lx ERROR\n", buf->raddr);

@@ -1905,6 +1902,7 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
 	struct fastrpc_buf *buf = NULL, *iter, *b;
 	struct fastrpc_req_munmap req;
 	struct device *dev = fl->sctx->dev;
+	int err;
 
 	if (copy_from_user(&req, argp, sizeof(req)))
 		return -EFAULT;

@@ -1912,6 +1910,7 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
 	spin_lock(&fl->lock);
 	list_for_each_entry_safe(iter, b, &fl->mmaps, node) {
 		if ((iter->raddr == req.vaddrout) && (iter->size == req.size)) {
+			list_del(&iter->node);
 			buf = iter;
 			break;
 		}

@@ -1924,7 +1923,14 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
 		return -EINVAL;
 	}
 
-	return fastrpc_req_munmap_impl(fl, buf);
+	err = fastrpc_req_munmap_impl(fl, buf);
+	if (err) {
+		spin_lock(&fl->lock);
+		list_add_tail(&buf->node, &fl->mmaps);
+		spin_unlock(&fl->lock);
+	}
+
+	return err;
 }
 
 static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)

2.53.0

From: Junrui Luo <[email protected]>

When an invoke is interrupted by a signal, wait_for_completion_interruptible() returns -ERESTARTSYS and fastrpc_internal_invoke() moves every buffer from fl->mmaps onto cctx->invoke_interrupted_mmaps. This list_del()/list_add_tail() walk runs without holding fl->lock, the lock that serialises fl->mmaps in fastrpc_req_mmap() and fastrpc_req_munmap() everywhere else.

Take fl->lock around the move, matching every other fl->mmaps accessor.

---
 drivers/misc/fastrpc.c | 2 ++
 1 file changed, 2 insertions(+)

index 12dcd2e737c3..c75eafe872cc 100644
@@ -1305,10 +1305,12 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl,  u32 kernel,
 	}
 
 	if (err == -ERESTARTSYS) {
+		spin_lock(&fl->lock);
 		list_for_each_entry_safe(buf, b, &fl->mmaps, node) {
 			list_del(&buf->node);
 			list_add_tail(&buf->node, &fl->cctx->invoke_interrupted_mmaps);
 		}
+		spin_unlock(&fl->lock);
 	}
 
 	if (err)

2.53.0

From: Anandu Krishnan E <[email protected]>

fastrpc_channel_ctx_get() is called in fastrpc_device_open() before fastrpc_session_alloc(). If session alloc fails, the error path returns -EBUSY without calling fastrpc_channel_ctx_put(), leaking the reference. Fix by adding the missing put.

---
 drivers/misc/fastrpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index c75eafe872cc..f8cbe30c5d5c 100644
@@ -1675,7 +1675,7 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp)
 		dev_err(&cctx->rpdev->dev, "No session available\n");
 		mutex_destroy(&fl->mutex);
 		kfree(fl);
-
+		fastrpc_channel_ctx_put(cctx);
 		return -EBUSY;
 	}
 

2.53.0

From: Eddie Lin <[email protected]>

The 'ctx_idr' is initialized but never destroyed when the channel context is freed, leading to a memory leak. Add idr_destroy() to properly clean up the IDR resources.

---
 drivers/misc/fastrpc.c | 1 +
 1 file changed, 1 insertion(+)

index f8cbe30c5d5c..eb6c2a78d3c7 100644
@@ -492,6 +492,7 @@ static void fastrpc_channel_ctx_free(struct kref *ref)
 
 	cctx = container_of(ref, struct fastrpc_channel_ctx, refcount);
 
+	idr_destroy(&cctx->ctx_idr);
 	kfree(cctx);
 }
 

2.53.0