[PATCH v3 3/9] misc: fastrpc: Fix memory corruption in DSP capabilities

Ekansh Gupta posted 9 patches 1 year, 6 months ago
There is a newer version of this series
[PATCH v3 3/9] misc: fastrpc: Fix memory corruption in DSP capabilities
Posted by Ekansh Gupta 1 year, 6 months ago
DSP capabilities request is sending bad size to utilities skel
call which is resulting in memory corruption. Pass proper size
to avoid the corruption.

Fixes: 6c16fd8bdd40 ("misc: fastrpc: Add support to get DSP capabilities")
Cc: stable <stable@kernel.org>
Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
---
 drivers/misc/fastrpc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 61389795f498..3e1ab58038ed 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1695,6 +1695,7 @@ static int fastrpc_get_info_from_dsp(struct fastrpc_user *fl, uint32_t *dsp_attr
 
 	/* Capability filled in userspace */
 	dsp_attr_buf[0] = 0;
+	dsp_attr_buf_len -= 1;
 
 	args[0].ptr = (u64)(uintptr_t)&dsp_attr_buf_len;
 	args[0].length = sizeof(dsp_attr_buf_len);
-- 
2.43.0
Re: [PATCH v3 3/9] misc: fastrpc: Fix memory corruption in DSP capabilities
Posted by Srinivas Kandagatla 1 year, 6 months ago

On 30/05/2024 11:20, Ekansh Gupta wrote:
> DSP capabilities request is sending bad size to utilities skel
What you exactly mean by this?

Curretly driver is sending 1024 bytes of buffer, why is DSP not happy 
with this size?

> call which is resulting in memory corruption. Pass proper size
What does proper size mean?
> to avoid the corruption.
> 
> Fixes: 6c16fd8bdd40 ("misc: fastrpc: Add support to get DSP capabilities")
> Cc: stable <stable@kernel.org>
> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> ---
>   drivers/misc/fastrpc.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 61389795f498..3e1ab58038ed 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -1695,6 +1695,7 @@ static int fastrpc_get_info_from_dsp(struct fastrpc_user *fl, uint32_t *dsp_attr
>   
>   	/* Capability filled in userspace */
>   	dsp_attr_buf[0] = 0;
> +	dsp_attr_buf_len -= 1;

is DSP expecting 255 *4 bytes instead of 256 *4?

--srini

>   
>   	args[0].ptr = (u64)(uintptr_t)&dsp_attr_buf_len;
>   	args[0].length = sizeof(dsp_attr_buf_len);
Re: [PATCH v3 3/9] misc: fastrpc: Fix memory corruption in DSP capabilities
Posted by Ekansh Gupta 1 year, 6 months ago
On 5/31/2024 3:06 PM, Srinivas Kandagatla wrote:
>
>
> On 30/05/2024 11:20, Ekansh Gupta wrote:
>> DSP capabilities request is sending bad size to utilities skel
> What you exactly mean by this?
>
> Curretly driver is sending 1024 bytes of buffer, why is DSP not happy 
> with this size?

Copying the comment sent to Dmitry's queries:
args[0] is expected to carry the information about the total number of attributes to be copied from DSP
and not the information about the size to be copied. Passing the size information leads to a failure
suggesting bad arguments passed to DSP.

>
>> call which is resulting in memory corruption. Pass proper size
> What does proper size mean?

args[1] is not carrying the information for full 256 sized array. It's sending the input argument as the first
index of the array, &dsp_attr_buf[1](resulting in total 255 size req), so sending 256 length will result in
copying of information out of the array which would result in problems

>> to avoid the corruption.
>>
>> Fixes: 6c16fd8bdd40 ("misc: fastrpc: Add support to get DSP 
>> capabilities")
>> Cc: stable <stable@kernel.org>
>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
>> ---
>>   drivers/misc/fastrpc.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index 61389795f498..3e1ab58038ed 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -1695,6 +1695,7 @@ static int fastrpc_get_info_from_dsp(struct 
>> fastrpc_user *fl, uint32_t *dsp_attr
>>         /* Capability filled in userspace */
>>       dsp_attr_buf[0] = 0;
>> +    dsp_attr_buf_len -= 1;
>
> is DSP expecting 255 *4 bytes instead of 256 *4?
DSP is expecting the information about the number of attributes to be 
updated, i.e., 255.
>
> --srini
>
>>         args[0].ptr = (u64)(uintptr_t)&dsp_attr_buf_len;
>>       args[0].length = sizeof(dsp_attr_buf_len);
Re: [PATCH v3 3/9] misc: fastrpc: Fix memory corruption in DSP capabilities
Posted by Dmitry Baryshkov 1 year, 6 months ago
On Thu, May 30, 2024 at 03:50:21PM +0530, Ekansh Gupta wrote:
> DSP capabilities request is sending bad size to utilities skel
> call which is resulting in memory corruption. Pass proper size
> to avoid the corruption.
> 
> Fixes: 6c16fd8bdd40 ("misc: fastrpc: Add support to get DSP capabilities")
> Cc: stable <stable@kernel.org>
> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>

Should be squashed to the previous commit.

> ---
>  drivers/misc/fastrpc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 61389795f498..3e1ab58038ed 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -1695,6 +1695,7 @@ static int fastrpc_get_info_from_dsp(struct fastrpc_user *fl, uint32_t *dsp_attr
>  
>  	/* Capability filled in userspace */
>  	dsp_attr_buf[0] = 0;
> +	dsp_attr_buf_len -= 1;
>  
>  	args[0].ptr = (u64)(uintptr_t)&dsp_attr_buf_len;
>  	args[0].length = sizeof(dsp_attr_buf_len);
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry