2023-10-13 12:21:02

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 0/4] misc: fastrpc: fixes for v6.6

From: Srinivas Kandagatla <[email protected]>

Hi Greg,

Here are some fixes in fastrpc driver mostly around cleanup path after
remote invoke.

Can you please apply them for v6.6

thanks,
Srini

Ekansh Gupta (4):
misc: fastrpc: Reset metadata buffer to avoid incorrect free
misc: fastrpc: Free DMA handles for RPC calls with no arguments
misc: fastrpc: Clean buffers on remote invocation failures
misc: fastrpc: Unmap only if buffer is unmapped from DSP

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

--
2.25.1


2023-10-13 12:21:02

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 4/4] misc: fastrpc: Unmap only if buffer is unmapped from DSP

From: Ekansh Gupta <[email protected]>

For unmapping any buffer from kernel, it should first be unmapped
from DSP. In case unmap from DSP request fails, the map should not
be removed from kernel as it might lead to SMMU faults and other
memory issues.

Fixes: 5c1b97c7d7b7 ("misc: fastrpc: add support for FASTRPC_IOCTL_MEM_MAP/UNMAP")
Cc: stable <[email protected]>
Signed-off-by: Ekansh Gupta <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/misc/fastrpc.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 3cdc58488db1..1c6c62a7f7f5 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1981,11 +1981,13 @@ static int fastrpc_req_mem_unmap_impl(struct fastrpc_user *fl, struct fastrpc_me
sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MEM_UNMAP, 1, 0);
err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc,
&args[0]);
- fastrpc_map_put(map);
- if (err)
+ if (err) {
dev_err(dev, "unmmap\tpt fd = %d, 0x%09llx error\n", map->fd, map->raddr);
+ return err;
+ }
+ fastrpc_map_put(map);

- return err;
+ return 0;
}

static int fastrpc_req_mem_unmap(struct fastrpc_user *fl, char __user *argp)
--
2.25.1

2023-10-13 12:21:10

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 3/4] misc: fastrpc: Clean buffers on remote invocation failures

From: Ekansh Gupta <[email protected]>

With current design, buffers and dma handles are not freed in case
of remote invocation failures returned from DSP. This could result
in buffer leakings and dma handle pointing to wrong memory in the
fastrpc kernel. Adding changes to clean buffers and dma handles
even when remote invocation to DSP returns failures.

Fixes: c68cfb718c8f ("misc: fastrpc: Add support for context Invoke method")
Cc: stable <[email protected]>
Signed-off-by: Ekansh Gupta <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/misc/fastrpc.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index a52701c1b018..3cdc58488db1 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1176,11 +1176,6 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
err = wait_for_completion_interruptible(&ctx->work);
}

- if (err)
- goto bail;
-
- /* Check the response from remote dsp */
- err = ctx->retval;
if (err)
goto bail;

@@ -1191,6 +1186,11 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
if (err)
goto bail;

+ /* Check the response from remote dsp */
+ err = ctx->retval;
+ if (err)
+ goto bail;
+
bail:
if (err != -ERESTARTSYS && err != -ETIMEDOUT) {
/* We are done with this compute context */
--
2.25.1

2023-10-13 12:21:16

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 2/4] misc: fastrpc: Free DMA handles for RPC calls with no arguments

From: Ekansh Gupta <[email protected]>

The FDs for DMA handles to be freed is updated in fdlist by DSP over
a remote call. This holds true even for remote calls with no
arguments. To handle this, get_args and put_args are needed to
be called for remote calls with no arguments also as fdlist
is allocated in get_args and FDs updated in fdlist is freed
in put_args.

Fixes: 8f6c1d8c4f0c ("misc: fastrpc: Add fdlist implementation")
Cc: stable <[email protected]>
Signed-off-by: Ekansh Gupta <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/misc/fastrpc.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index fb921975b56d..a52701c1b018 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1091,6 +1091,7 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
}
}

+ /* Clean up fdlist which is updated by DSP */
for (i = 0; i < FASTRPC_MAX_FDLIST; i++) {
if (!fdlist[i])
break;
@@ -1157,11 +1158,9 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
if (IS_ERR(ctx))
return PTR_ERR(ctx);

- if (ctx->nscalars) {
- err = fastrpc_get_args(kernel, ctx);
- if (err)
- goto bail;
- }
+ err = fastrpc_get_args(kernel, ctx);
+ if (err)
+ goto bail;

/* make sure that all CPU memory writes are seen by DSP */
dma_wmb();
@@ -1185,14 +1184,12 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
if (err)
goto bail;

- if (ctx->nscalars) {
- /* make sure that all memory writes by DSP are seen by CPU */
- dma_rmb();
- /* populate all the output buffers with results */
- err = fastrpc_put_args(ctx, kernel);
- if (err)
- goto bail;
- }
+ /* make sure that all memory writes by DSP are seen by CPU */
+ dma_rmb();
+ /* populate all the output buffers with results */
+ err = fastrpc_put_args(ctx, kernel);
+ if (err)
+ goto bail;

bail:
if (err != -ERESTARTSYS && err != -ETIMEDOUT) {
--
2.25.1

2023-10-13 12:21:49

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 1/4] misc: fastrpc: Reset metadata buffer to avoid incorrect free

From: Ekansh Gupta <[email protected]>

Metadata buffer is allocated during get_args for any remote call.
This buffer carries buffers, fdlists and other payload information
for the call. If the buffer is not reset, put_args might find some
garbage FDs in the fdlist which might have an existing mapping in
the list. This could result in improper freeing of FD map when DSP
might still be using the buffer. Added change to reset the metadata
buffer after allocation.

Fixes: 8f6c1d8c4f0c ("misc: fastrpc: Add fdlist implementation")
Cc: stable <[email protected]>
Signed-off-by: Ekansh Gupta <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/misc/fastrpc.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index a66b7c111cd5..fb921975b56d 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -958,6 +958,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
if (err)
return err;

+ memset(ctx->buf->virt, 0, pkt_size);
rpra = ctx->buf->virt;
list = fastrpc_invoke_buf_start(rpra, ctx->nscalars);
pages = fastrpc_phy_page_start(list, ctx->nscalars);
--
2.25.1