For registered buffers, fastrpc driver sends the buffer information
to remote subsystem. There is a problem with current implementation
where the page address is being sent with an offset leading to
improper buffer address on DSP. This is leads to functional failures
as DSP expects base address in page information and extracts offset
information from remote arguments. Mask the offset and pass the base
page address to DSP.
Fixes: 80f3afd72bd4 ("misc: fastrpc: consider address offset before sending to DSP")
Cc: stable <stable@kernel.org>
Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
---
drivers/misc/fastrpc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 48d08eeb2d20..cfa1546c9e3f 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -992,7 +992,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
mmap_read_lock(current->mm);
vma = find_vma(current->mm, ctx->args[i].ptr);
if (vma)
- pages[i].addr += ctx->args[i].ptr -
+ pages[i].addr += (ctx->args[i].ptr & PAGE_MASK) -
vma->vm_start;
mmap_read_unlock(current->mm);
--
2.34.1On Wed, Dec 18, 2024 at 03:54:28PM +0530, Ekansh Gupta wrote:
> For registered buffers, fastrpc driver sends the buffer information
> to remote subsystem. There is a problem with current implementation
> where the page address is being sent with an offset leading to
> improper buffer address on DSP. This is leads to functional failures
> as DSP expects base address in page information and extracts offset
> information from remote arguments. Mask the offset and pass the base
> page address to DSP.
>
> Fixes: 80f3afd72bd4 ("misc: fastrpc: consider address offset before sending to DSP")
This was committed in 2019. Are you saying that the driver has been
broken since that time? If so, what is the impact? Because I've
definitely been running fastrpc workload after that moment.
Also, is there any reason for neglecting checkpatch warning?
> Cc: stable <stable@kernel.org>
> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> ---
> drivers/misc/fastrpc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 48d08eeb2d20..cfa1546c9e3f 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -992,7 +992,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> mmap_read_lock(current->mm);
> vma = find_vma(current->mm, ctx->args[i].ptr);
> if (vma)
> - pages[i].addr += ctx->args[i].ptr -
> + pages[i].addr += (ctx->args[i].ptr & PAGE_MASK) -
> vma->vm_start;
> mmap_read_unlock(current->mm);
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
On 12/18/2024 4:42 PM, Dmitry Baryshkov wrote:
> On Wed, Dec 18, 2024 at 03:54:28PM +0530, Ekansh Gupta wrote:
>> For registered buffers, fastrpc driver sends the buffer information
>> to remote subsystem. There is a problem with current implementation
>> where the page address is being sent with an offset leading to
>> improper buffer address on DSP. This is leads to functional failures
>> as DSP expects base address in page information and extracts offset
>> information from remote arguments. Mask the offset and pass the base
>> page address to DSP.
>>
>> Fixes: 80f3afd72bd4 ("misc: fastrpc: consider address offset before sending to DSP")
> This was committed in 2019. Are you saying that the driver has been
> broken since that time? If so, what is the impact? Because I've
> definitely been running fastrpc workload after that moment.
>
> Also, is there any reason for neglecting checkpatch warning?
Hi Dmitry,
This issue is observed is a corner case when some buffer which is registered with fastrpc
framework is passed with some offset by user and then the DSP implementation tried to
read the data. As DSP expects base address and takes care of offsetting with remote
arguments, passing an offsetted address will result in some unexpected data read in DSP.
All generic usecases usually pass the buffer as it is hence is problem is not usually observed. If
someone tries to pass offsetted buffer and then tries to compare data at HLOS and DSP end,
then the ambiguity will be observed.
Apologies for delay in response as I was traveling with very limited internet access.
--ekansh
>
>> Cc: stable <stable@kernel.org>
>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
>> ---
>> drivers/misc/fastrpc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index 48d08eeb2d20..cfa1546c9e3f 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -992,7 +992,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>> mmap_read_lock(current->mm);
>> vma = find_vma(current->mm, ctx->args[i].ptr);
>> if (vma)
>> - pages[i].addr += ctx->args[i].ptr -
>> + pages[i].addr += (ctx->args[i].ptr & PAGE_MASK) -
>> vma->vm_start;
>> mmap_read_unlock(current->mm);
>>
>> --
>> 2.34.1
>>
On Thu, Jan 09, 2025 at 11:09:30AM +0530, Ekansh Gupta wrote:
>
>
>
> On 12/18/2024 4:42 PM, Dmitry Baryshkov wrote:
> > On Wed, Dec 18, 2024 at 03:54:28PM +0530, Ekansh Gupta wrote:
> >> For registered buffers, fastrpc driver sends the buffer information
> >> to remote subsystem. There is a problem with current implementation
> >> where the page address is being sent with an offset leading to
> >> improper buffer address on DSP. This is leads to functional failures
> >> as DSP expects base address in page information and extracts offset
> >> information from remote arguments. Mask the offset and pass the base
> >> page address to DSP.
> >>
> >> Fixes: 80f3afd72bd4 ("misc: fastrpc: consider address offset before sending to DSP")
> > This was committed in 2019. Are you saying that the driver has been
> > broken since that time? If so, what is the impact? Because I've
> > definitely been running fastrpc workload after that moment.
> >
> > Also, is there any reason for neglecting checkpatch warning?
> Hi Dmitry,
>
> This issue is observed is a corner case when some buffer which is registered with fastrpc
> framework is passed with some offset by user and then the DSP implementation tried to
> read the data. As DSP expects base address and takes care of offsetting with remote
> arguments, passing an offsetted address will result in some unexpected data read in DSP.
>
> All generic usecases usually pass the buffer as it is hence is problem is not usually observed. If
> someone tries to pass offsetted buffer and then tries to compare data at HLOS and DSP end,
> then the ambiguity will be observed.
Ok. Thanks for the explanation. Please consider moving relevant bits to
the commit message.
Also this brings up a topic that we have discussed several times: what
is the progress on a testsuite for the API?
Last, but not least, does this issue result in a possible access to
unrelated memory areas? Can it be exploited somehow?
>
> Apologies for delay in response as I was traveling with very limited internet access.
>
> --ekansh
> >
> >> Cc: stable <stable@kernel.org>
> >> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> >> ---
> >> drivers/misc/fastrpc.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> >> index 48d08eeb2d20..cfa1546c9e3f 100644
> >> --- a/drivers/misc/fastrpc.c
> >> +++ b/drivers/misc/fastrpc.c
> >> @@ -992,7 +992,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> >> mmap_read_lock(current->mm);
> >> vma = find_vma(current->mm, ctx->args[i].ptr);
> >> if (vma)
> >> - pages[i].addr += ctx->args[i].ptr -
> >> + pages[i].addr += (ctx->args[i].ptr & PAGE_MASK) -
> >> vma->vm_start;
Shouldn't it be other way around:
pages[i].addr += (ctx->args[i].ptr - vma->vm_start) & PAGE_MASK;
Also, can offset be larger than a page size?
> >> mmap_read_unlock(current->mm);
> >>
> >> --
> >> 2.34.1
> >>
>
--
With best wishes
Dmitry
© 2016 - 2025 Red Hat, Inc.