The buffer list is sorted and this is not being
considered while calculating packet size. This
would lead to improper copy length calculation
for non-dmaheap buffers which would eventually
cause sending improper buffers to DSP.
Signed-off-by: Jeya R <[email protected]>
---
drivers/misc/fastrpc.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index beda610..a7e550f 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -719,16 +719,21 @@ static int fastrpc_get_meta_size(struct fastrpc_invoke_ctx *ctx)
static u64 fastrpc_get_payload_size(struct fastrpc_invoke_ctx *ctx, int metalen)
{
u64 size = 0;
- int i;
+ int oix = 0;
size = ALIGN(metalen, FASTRPC_ALIGN);
- for (i = 0; i < ctx->nscalars; i++) {
+ for (oix = 0; oix < ctx->nbufs; oix++) {
+ int i = ctx->olaps[oix].raix;
+
+ if (ctx->args[i].length == 0)
+ continue;
+
if (ctx->args[i].fd == 0 || ctx->args[i].fd == -1) {
- if (ctx->olaps[i].offset == 0)
+ if (ctx->olaps[oix].offset == 0)
size = ALIGN(size, FASTRPC_ALIGN);
- size += (ctx->olaps[i].mend - ctx->olaps[i].mstart);
+ size += (ctx->olaps[oix].mend - ctx->olaps[oix].mstart);
}
}
--
2.7.4
On Mon, Sep 20, 2021 at 01:45:31PM +0530, Jeya R wrote:
> The buffer list is sorted and this is not being
> considered while calculating packet size. This
> would lead to improper copy length calculation
> for non-dmaheap buffers which would eventually
> cause sending improper buffers to DSP.
You do have the full 72 columns to use :)
>
> Signed-off-by: Jeya R <[email protected]>
> ---
> drivers/misc/fastrpc.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
What commit does this fix?
thanks,
greg k-h
On 2021-09-20 14:38, Greg KH wrote:
> On Mon, Sep 20, 2021 at 01:45:31PM +0530, Jeya R wrote:
>> The buffer list is sorted and this is not being
>> considered while calculating packet size. This
>> would lead to improper copy length calculation
>> for non-dmaheap buffers which would eventually
>> cause sending improper buffers to DSP.
>
> You do have the full 72 columns to use :)
Thanks, will update the commit message considering this.
>
>>
>> Signed-off-by: Jeya R <[email protected]>
>> ---
>> drivers/misc/fastrpc.c | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> What commit does this fix?
>
> thanks,
>
> greg k-h
Payload calculation function was modified to handle buffer overlapping
calculation in this commit:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/misc/fastrpc.c?h=v5.15-rc2&id=25e8dfb83cda0a123bb1e091d6c3599cde050d76
Here during buffer overlap calculation, the buffer list is getting
sorted. This needs to be considered during the calculation of payload
size also by using unsorted buffer index "raix".
On Mon, Sep 20, 2021 at 03:01:07PM +0530, [email protected] wrote:
> On 2021-09-20 14:38, Greg KH wrote:
> > On Mon, Sep 20, 2021 at 01:45:31PM +0530, Jeya R wrote:
> > > The buffer list is sorted and this is not being
> > > considered while calculating packet size. This
> > > would lead to improper copy length calculation
> > > for non-dmaheap buffers which would eventually
> > > cause sending improper buffers to DSP.
> >
> > You do have the full 72 columns to use :)
>
> Thanks, will update the commit message considering this.
>
> >
> > >
> > > Signed-off-by: Jeya R <[email protected]>
> > > ---
> > > drivers/misc/fastrpc.c | 13 +++++++++----
> > > 1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > What commit does this fix?
> >
> > thanks,
> >
> > greg k-h
>
> Payload calculation function was modified to handle buffer overlapping
> calculation in this commit:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/misc/fastrpc.c?h=v5.15-rc2&id=25e8dfb83cda0a123bb1e091d6c3599cde050d76
>
> Here during buffer overlap calculation, the buffer list is getting sorted.
> This needs to be considered during the calculation of payload size also by
> using unsorted buffer index "raix".
Ok, then please add a "Fixes:" tag when you resend this.
thanks,
greg k-h
Thanks Jeya for the fix,
On 20/09/2021 09:15, Jeya R wrote:
> The buffer list is sorted and this is not being
> considered while calculating packet size. This
> would lead to improper copy length calculation
> for non-dmaheap buffers which would eventually
> cause sending improper buffers to DSP.
>
we need below fixes tag:
Fixes: c68cfb718c8f ("misc: fastrpc: Add support for context Invoke method")
> Signed-off-by: Jeya R <[email protected]>
> ---
> drivers/misc/fastrpc.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index beda610..a7e550f 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -719,16 +719,21 @@ static int fastrpc_get_meta_size(struct fastrpc_invoke_ctx *ctx)
> static u64 fastrpc_get_payload_size(struct fastrpc_invoke_ctx *ctx, int metalen)
> {
> u64 size = 0;
> - int i;
> + int oix = 0;
Unnecessary initialization here.
>
> size = ALIGN(metalen, FASTRPC_ALIGN);
> - for (i = 0; i < ctx->nscalars; i++) {
> + for (oix = 0; oix < ctx->nbufs; oix++) {
> + int i = ctx->olaps[oix].raix;
> +
[--
> + if (ctx->args[i].length == 0)
> + continue;
> +
--]
This additional check will not really have any effect on size
calculations, as (ctx->olaps[oix].mend - ctx->olaps[oix].mstart) would
result in zero.
Any reason why this check was added?
--srini
> if (ctx->args[i].fd == 0 || ctx->args[i].fd == -1) {
>
> - if (ctx->olaps[i].offset == 0)
> + if (ctx->olaps[oix].offset == 0)
> size = ALIGN(size, FASTRPC_ALIGN);
>
> - size += (ctx->olaps[i].mend - ctx->olaps[i].mstart);
> + size += (ctx->olaps[oix].mend - ctx->olaps[oix].mstart);
> }
> }
>
>