On 22/02/2021 13:53, Jonathan Marek wrote:
On 2/22/21 7:37 AM, Srinivas Kandagatla wrote:
On 18/02/2021 03:20, Jonathan Marek wrote:
fastrpc_internal_invoke() will call fastrpc_map_create, so there is no
point in having it called here. This does change the behavior
somewhat as
fastrpc_internal_invoke() will release the map afterwards, but that's
what
we want to happen in this case.
This will crash the DSP as you will be freeing the init process memory
while it is actively using it!
The shell/init process is created as part of user process and it
should be valid until the user process is valid! We can not free it
when the invoke is finished/acked as we normally do for other invoke
context!
In some firmwares the shell process is statically built into the DSP
firmware which might work! But other normal cases are totally broken
by this patch!
--srini
I am not using the static guest process, I am using the
FASTRPC_IOCTL_INIT_CREATE to load a fastrpc shell process. It doesn't
crash.
AFAIK the DSP does not need the process memory after the process
creation - this would allow userspace to modify the executable after the
This process memory includes memory to load shell and the heap(complete
process memory) so it should be kept till the process dies!
--srini
DSP verifies the hash/signature. So the DSP absolutely needs to make a
copy of it before verifying it (otherwise this would be a pretty serious
and obvious security flaw in qcom's fastrpc system. but I wouldn't be
surprised!).
Signed-off-by: Jonathan Marek <jonathan@xxxxxxxx>
---
drivers/misc/fastrpc.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 170352b43ab6..ccad9f5f5e2f 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1013,7 +1013,6 @@ static int fastrpc_init_create_process(struct
fastrpc_user *fl,
struct fastrpc_init_create init;
struct fastrpc_invoke_args *args;
struct fastrpc_phy_page pages[1];
- struct fastrpc_map *map = NULL;
struct fastrpc_buf *imem = NULL;
int memlen;
int err;
@@ -1049,18 +1048,12 @@ static int fastrpc_init_create_process(struct
fastrpc_user *fl,
inbuf.siglen = init.siglen;
fl->pd = USER_PD;
- if (init.filelen && init.filefd) {
- err = fastrpc_map_create(fl, init.filefd, init.filelen, &map);
- if (err)
- goto err;
- }
- > memlen = ALIGN(max(INIT_FILELEN_MAX, (int)init.filelen * 4),
1024 * 1024);
err = fastrpc_buf_alloc(fl, fl->sctx->dev, memlen,
&imem);
if (err)
- goto err_alloc;
+ goto err;
fl->init_mem = imem;
args[0].ptr = (u64)(uintptr_t)&inbuf;
@@ -1106,9 +1099,6 @@ static int fastrpc_init_create_process(struct
fastrpc_user *fl,
err_invoke:
fl->init_mem = NULL;
fastrpc_buf_free(imem);
-err_alloc:
- if (map)
- fastrpc_map_put(map);
err:
kfree(args);
