misc: fastrpc: fix memory corruption

The fastrpc driver uses a fixed-sized array to store its sessions but missing and broken sanity checks could lead to memory beyond the array being corrupted.

This specifically happens on SC8280XP platforms that use 14 sessions for the compute DSP.

These are all needed for 6.0.

Johan

Johan Hovold (3):
misc: fastrpc: fix memory corruption on probe
misc: fastrpc: fix memory corruption on open
misc: fastrpc: increase maximum session count

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

2.35.1

Add the missing sanity check on the probed-session count to avoid corrupting memory beyond the fixed-size slab-allocated session array when there are more than FASTRPC_MAX_SESSIONS sessions defined in the devicetree.

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

index 93ebd174d848..88091778c1b8 100644
@@ -1943,6 +1943,11 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
 	of_property_read_u32(dev->of_node, "qcom,nsessions", &sessions);
 
 	spin_lock_irqsave(&cctx->lock, flags);
+	if (cctx->sesscount >= FASTRPC_MAX_SESSIONS) {
+		dev_err(&pdev->dev, "too many sessions\n");
+		spin_unlock_irqrestore(&cctx->lock, flags);
+		return -ENOSPC;
+	}
 	sess = &cctx->session[cctx->sesscount];
 	sess->used = false;
 	sess->valid = true;

2.35.1

The probe session-duplication overflow check incremented the session count also when there were no more available sessions so that memory beyond the fixed-size slab-allocated session array could be corrupted in fastrpc_session_alloc() on open().

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

index 88091778c1b8..6e312ac85668 100644
@@ -1948,7 +1948,7 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
 		spin_unlock_irqrestore(&cctx->lock, flags);
 		return -ENOSPC;
 	}
-	sess = &cctx->session[cctx->sesscount];
+	sess = &cctx->session[cctx->sesscount++];
 	sess->used = false;
 	sess->valid = true;
 	sess->dev = dev;

@@ -1961,13 +1961,12 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
 		struct fastrpc_session_ctx *dup_sess;
 
 		for (i = 1; i < sessions; i++) {
-			if (cctx->sesscount++ >= FASTRPC_MAX_SESSIONS)
+			if (cctx->sesscount >= FASTRPC_MAX_SESSIONS)
 				break;
-			dup_sess = &cctx->session[cctx->sesscount];
+			dup_sess = &cctx->session[cctx->sesscount++];
 			memcpy(dup_sess, sess, sizeof(*dup_sess));
 		}
 	}
-	cctx->sesscount++;
 	spin_unlock_irqrestore(&cctx->lock, flags);
 	rc = dma_set_mask(dev, DMA_BIT_MASK(32));
 	if (rc) {

2.35.1

The SC8280XP platform uses 14 sessions for the compute DSP so increment the maximum session count.

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

index 6e312ac85668..5d9e3483b89d 100644
@@ -25,7 +25,7 @@
 #define SDSP_DOMAIN_ID (2)
 #define CDSP_DOMAIN_ID (3)
 #define FASTRPC_DEV_MAX		4 /* adsp, mdsp, slpi, cdsp*/
-#define FASTRPC_MAX_SESSIONS	13 /*12 compute, 1 cpz*/
+#define FASTRPC_MAX_SESSIONS	14
 #define FASTRPC_MAX_VMIDS	16
 #define FASTRPC_ALIGN		128
 #define FASTRPC_MAX_FDLIST	16

2.35.1

Hi Johan,

Thanks for doing this.

I see that we hit this issue once again, and the way we are fixing it is not really scalable. We should really get rid of FASTRPC_MAX_SESSIONS.

We should allocate the sessions dynamically based in the child node count and qcom,nsessions.

thanks,
Srini

Yeah, I was a bit surprised to find that the underlying bugs (i.e. the incomplete sanity checks) weren't fixed the last time this memory corruption was reported:

	https://lore.kernel.org/all/[email protected]/

That sounds like it would be an improvement.

But at least now you'll get an error message during probe rather than silent memory corruption when bringing up a new SoC that needs more than the current maximum number of sessions.

Johan