linux-kernel - Re: [PATCH v1 1/2] misc: fastrpc: Fix registered buffer page address

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <q4h6myikxgg226kbalquvoiveez2cpipopod5rt45d37f46fev@v5ir6c5eg343>
Date: Thu, 9 Jan 2025 14:25:35 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Ekansh Gupta <quic_ekangupt@...cinc.com>
Cc: srinivas.kandagatla@...aro.org, linux-arm-msm@...r.kernel.org, 
	gregkh@...uxfoundation.org, quic_bkumar@...cinc.com, linux-kernel@...r.kernel.org, 
	quic_chennak@...cinc.com, dri-devel@...ts.freedesktop.org, arnd@...db.de, 
	stable <stable@...nel.org>
Subject: Re: [PATCH v1 1/2] misc: fastrpc: Fix registered buffer page address

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@...nel.org>
> >> Signed-off-by: Ekansh Gupta <quic_ekangupt@...cinc.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