fastrpc_rpmsg_remove() clears cctx->rpdev, wakes every pending invoke
with -EPIPE through fastrpc_notify_users(), and then goes straight on to
of_platform_depopulate() the context bank devices, with nothing
synchronising the two:
- The only thing stopping new work is the unlocked rpdev check at the
top of fastrpc_internal_invoke(). A thread can observe a valid rpdev,
get preempted, and resume in the middle of the teardown.
- Invokes that were already admitted are woken and head for the bail:
label, where fastrpc_context_put() releases ctx->buf through
dma_free_coherent(). If the depopulate wins, the call runs against an
unbound context bank and falls through to dma_direct_free(), which
treats the IOVA as a physical address.
Close both windows under cctx->lock:
- Add cctx->teardown, set at the start of fastrpc_rpmsg_remove().
fastrpc_internal_invoke() checks it and, in the same critical
section, increments cctx->invoke_cnt, so an invoke is either rejected
or counted. The count is dropped on every return path.
- fastrpc_rpmsg_remove() waits for invoke_cnt to reach zero before
clearing rpdev and before touching any other channel resource.
The gate lives in fastrpc_internal_invoke() rather than in the ioctl
dispatcher so that it also covers fastrpc_device_release() ->
fastrpc_release_current_dsp_process(), which sends INIT_RELEASE outside
of any ioctl.
Teardown is kept separate from rpdev because the two have different
lifetimes: the gate must close immediately, whereas rpdev has to stay
valid until the admitted invokes have finished rpmsg_send(). Clearing
rpdev up front, as the code does today, lets an invoke that was sleeping
in an allocation dereference a NULL rpdev:
Unable to handle kernel NULL pointer dereference at virtual address
0000000000000358
CPU: 2 UID: 0 PID: 2003 Comm: adsprpcd
pc : fastrpc_internal_invoke+0xb40/0x1398 [fastrpc]
Call trace:
fastrpc_internal_invoke [fastrpc]
fastrpc_device_release [fastrpc]
__fput
__arm64_sys_close
The faulting address is the offset of ept within struct rpmsg_device,
i.e. the cctx->rpdev->ept dereference in rpmsg_send().
Signed-off-by: Jianping Li <[email protected]>
---
drivers/misc/fastrpc.c | 60 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 56 insertions(+), 4 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index c54c450cb571..3036925632d0 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -328,6 +328,15 @@ struct fastrpc_channel_ctx {
atomic_t ctx_seq;
u64 dma_mask;
const struct fastrpc_soc_data *soc_data;
+ /*
+ * Set once fastrpc_rpmsg_remove() starts tearing the channel down.
+ * Checked under @lock so that no new invoke can begin afterwards.
+ */
+ atomic_t teardown;
+ /* Number of in-flight invokes; guarded by @lock */
+ int invoke_cnt;
+ /* Woken whenever @invoke_cnt drops to zero */
+ wait_queue_head_t ssr_wait_queue;
};
struct fastrpc_device {
@@ -1350,12 +1359,23 @@ static int fastrpc_wait_for_completion(struct
fastrpc_invoke_ctx *ctx,
return fastrpc_wait_for_response(ctx, kernel);
}
+/* Caller must hold cctx->lock */
+static void fastrpc_channel_update_invoke_cnt(struct fastrpc_channel_ctx *cctx,
+ bool enter)
+{
+ if (enter)
+ cctx->invoke_cnt++;
+ else if (--cctx->invoke_cnt == 0)
+ wake_up(&cctx->ssr_wait_queue);
+}
+
static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
u32 handle, u32 sc,
struct fastrpc_invoke_args *args)
{
struct fastrpc_invoke_ctx *ctx = NULL;
struct fastrpc_buf *buf, *b;
+ unsigned long flags;
int err = 0;
@@ -1365,14 +1385,25 @@ static int fastrpc_internal_invoke(struct fastrpc_user
*fl, u32 kernel,
if (!fl->cctx->rpdev)
return -EPIPE;
+ spin_lock_irqsave(&fl->cctx->lock, flags);
+ if (atomic_read(&fl->cctx->teardown)) {
+ spin_unlock_irqrestore(&fl->cctx->lock, flags);
+ return -EPIPE;
+ }
+ fastrpc_channel_update_invoke_cnt(fl->cctx, true);
+ spin_unlock_irqrestore(&fl->cctx->lock, flags);
+
if (handle == FASTRPC_INIT_HANDLE && !kernel) {
dev_warn_ratelimited(fl->sctx->dev, "user app trying to send a
kernel RPC message (%d)\n", handle);
- return -EPERM;
+ err = -EPERM;
+ goto out;
}
ctx = fastrpc_context_alloc(fl, kernel, sc, args);
- if (IS_ERR(ctx))
- return PTR_ERR(ctx);
+ if (IS_ERR(ctx)) {
+ err = PTR_ERR(ctx);
+ goto out;
+ }
err = fastrpc_get_args(kernel, ctx);
if (err)
@@ -1428,6 +1459,10 @@ static int fastrpc_internal_invoke(struct fastrpc_user
*fl, u32 kernel,
if (err)
dev_dbg(fl->sctx->dev, "Error: Invoke Failed %d\n", err);
+out:
+ spin_lock_irqsave(&fl->cctx->lock, flags);
+ fastrpc_channel_update_invoke_cnt(fl->cctx, false);
+ spin_unlock_irqrestore(&fl->cctx->lock, flags);
return err;
}
@@ -2635,6 +2670,9 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
INIT_LIST_HEAD(&data->invoke_interrupted_mmaps);
spin_lock_init(&data->lock);
idr_init(&data->ctx_idr);
+ atomic_set(&data->teardown, 0);
+ data->invoke_cnt = 0;
+ init_waitqueue_head(&data->ssr_wait_queue);
data->domain_id = domain_id;
data->rpdev = rpdev;
dev_set_drvdata(&rpdev->dev, data);
@@ -2678,11 +2716,25 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device
*rpdev)
/* No invocations past this point */
spin_lock_irqsave(&cctx->lock, flags);
- cctx->rpdev = NULL;
+ atomic_set(&cctx->teardown, 1);
list_for_each_entry(user, &cctx->users, user)
fastrpc_notify_users(user);
spin_unlock_irqrestore(&cctx->lock, flags);
+ /*
+ * Wait for every invoke that was already past the gate to finish.
+ * They have all just been woken with -EPIPE, and no new one can be
+ * counted, so this is guaranteed to make progress.
+ */
+ spin_lock_irqsave(&cctx->lock, flags);
+ while (cctx->invoke_cnt > 0) {
+ spin_unlock_irqrestore(&cctx->lock, flags);
+ wait_event(cctx->ssr_wait_queue, cctx->invoke_cnt == 0);
+ spin_lock_irqsave(&cctx->lock, flags);
+ }
+ cctx->rpdev = NULL;
+ spin_unlock_irqrestore(&cctx->lock, flags);
+
if (cctx->fdevice)
misc_deregister(&cctx->fdevice->miscdev);
--
2.43.0
