Re: [PATCH v3 2/9] misc: fastrpc: Fix DSP capabilities request

From: Srinivas Kandagatla
Date: Fri May 31 2024 - 05:33:43 EST




On 30/05/2024 11:20, Ekansh Gupta wrote:
Incorrect remote arguments are getting passed when requesting for
capabilities from DSP. Also there is no requirement to update the
PD type as it might cause problems for any PD other than user PD.
In addition to this, the collected capability information is not
getting copied properly to user. Add changes to address these
problems and get correct DSP capabilities.

Fixes: 6c16fd8bdd40 ("misc: fastrpc: Add support to get DSP capabilities")
Cc: stable <stable@xxxxxxxxxx>
Signed-off-by: Ekansh Gupta <quic_ekangupt@xxxxxxxxxxx>
---
drivers/misc/fastrpc.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 4028cb96bcf2..61389795f498 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1700,9 +1700,8 @@ static int fastrpc_get_info_from_dsp(struct fastrpc_user *fl, uint32_t *dsp_attr
args[0].length = sizeof(dsp_attr_buf_len);
args[0].fd = -1;
args[1].ptr = (u64)(uintptr_t)&dsp_attr_buf[1];
- args[1].length = dsp_attr_buf_len;
+ args[1].length = dsp_attr_buf_len * sizeof(uint32_t);
This does not look correct,

we have allocated buffer of size FASTRPC_MAX_DSP_ATTRIBUTES_LEN which is
already (sizeof(u32) * FASTRPC_MAX_DSP_ATTRIBUTES)

now this patch multiplies with again sizeof(uint32_t), this is going to send dsp incorrect size for buffer and overrun the buffer size.



args[1].fd = -1;
- fl->pd = USER_PD;
another patch may be.

return fastrpc_internal_invoke(fl, true, FASTRPC_DSP_UTILITIES_HANDLE,
FASTRPC_SCALARS(0, 1, 1), args);
@@ -1730,7 +1729,7 @@ static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap,
if (!dsp_attributes)
return -ENOMEM;
- err = fastrpc_get_info_from_dsp(fl, dsp_attributes, FASTRPC_MAX_DSP_ATTRIBUTES_LEN);
+ err = fastrpc_get_info_from_dsp(fl, dsp_attributes, FASTRPC_MAX_DSP_ATTRIBUTES);

You change this again to send FASTRPC_MAX_DSP_ATTRIBUTES instead of FASTRPC_MAX_DSP_ATTRIBUTES_LEN but why?


if (err == DSP_UNSUPPORTED_API) {
dev_info(&cctx->rpdev->dev,
"Warning: DSP capabilities not supported on domain: %d\n", domain);
@@ -1783,7 +1782,7 @@ static int fastrpc_get_dsp_info(struct fastrpc_user *fl, char __user *argp)
if (err)
return err;
- if (copy_to_user(argp, &cap.capability, sizeof(cap.capability)))
+ if (copy_to_user(argp, &cap, sizeof(cap)))

Why are we copying the full struct here? All that user needs is cap.capability?



--srini


return -EFAULT;
return 0;