1. Patchew
  2. linux
  3. View series

[PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe

Xingjing Deng posted 1 patch 3 weeks ago
There is a newer version of this series
drivers/misc/fastrpc.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
[PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe
Posted by Xingjing Deng 3 weeks ago 
In the SDSP probe path, qcom_scm_assign_mem() is used to assign the
reserved memory to the configured VMIDs, but its return value was not
checked.

Fail the probe if the SCM call fails to avoid continuing with an
unexpected/incorrect memory permission configuration.

The file has passed the check of checkpatch.

Fixes: c3c0363bc72d4 ("misc: fastrpc: support complete DMA pool access to the DSP")
Cc: stable@vger.kernel.org # 6.11-rc1
Signed-off-by: Xingjing Deng <xjdeng@buaa.edu.cn>
---
v5:
- Squash the functional change and indentation fix into a single patch.
- Link to v4: https://lore.kernel.org/linux-arm-msm/2026011637-statute-showy-2c3f@gregkh/T/#t

v4:
- Format the indentation
- Link to v3: https://lore.kernel.org/linux-arm-msm/20260113084352.72itrloj5w7qb5o3@hu-mojha-hyd.qualcomm.com/T/#t

v3:
- Add missing linux-kernel@vger.kernel.org to cc list.
- Standarlize changelog placement/format.
- Link to v2: https://lore.kernel.org/linux-arm-msm/20260113063618.e2ke47gy3hnfi67e@hu-mojha-hyd.qualcomm.com/T/#t

v2:
- Add Fixes: and Cc: stable tags.
- Link to v1: https://lore.kernel.org/linux-arm-msm/20260113022550.4029635-1-xjdeng@buaa.edu.cn/T/#u
---
 drivers/misc/fastrpc.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index fb3b54e05928..d9650efa443f 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -2338,8 +2338,13 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 		if (!err) {
 			src_perms = BIT(QCOM_SCM_VMID_HLOS);
 
-			qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
-				    data->vmperms, data->vmcount);
+			err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
+					data->vmperms, data->vmcount);
+			if (err) {
+				dev_err(rdev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
+				    res.start, resource_size(&res), err);
+				goto err_free_data;
+			}
 		}
 
 	}
-- 
2.25.1
Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe
Posted by Greg KH 1 week, 5 days ago 
On Sat, Jan 17, 2026 at 10:03:51PM +0800, Xingjing Deng wrote:
> In the SDSP probe path, qcom_scm_assign_mem() is used to assign the
> reserved memory to the configured VMIDs, but its return value was not
> checked.
> 
> Fail the probe if the SCM call fails to avoid continuing with an
> unexpected/incorrect memory permission configuration.
> 
> The file has passed the check of checkpatch.
> 
> Fixes: c3c0363bc72d4 ("misc: fastrpc: support complete DMA pool access to the DSP")
> Cc: stable@vger.kernel.org # 6.11-rc1
> Signed-off-by: Xingjing Deng <xjdeng@buaa.edu.cn>
> ---
> v5:
> - Squash the functional change and indentation fix into a single patch.
> - Link to v4: https://lore.kernel.org/linux-arm-msm/2026011637-statute-showy-2c3f@gregkh/T/#t
> 
> v4:
> - Format the indentation
> - Link to v3: https://lore.kernel.org/linux-arm-msm/20260113084352.72itrloj5w7qb5o3@hu-mojha-hyd.qualcomm.com/T/#t
> 
> v3:
> - Add missing linux-kernel@vger.kernel.org to cc list.
> - Standarlize changelog placement/format.
> - Link to v2: https://lore.kernel.org/linux-arm-msm/20260113063618.e2ke47gy3hnfi67e@hu-mojha-hyd.qualcomm.com/T/#t
> 
> v2:
> - Add Fixes: and Cc: stable tags.
> - Link to v1: https://lore.kernel.org/linux-arm-msm/20260113022550.4029635-1-xjdeng@buaa.edu.cn/T/#u
> ---
>  drivers/misc/fastrpc.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index fb3b54e05928..d9650efa443f 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2338,8 +2338,13 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  		if (!err) {
>  			src_perms = BIT(QCOM_SCM_VMID_HLOS);
>  
> -			qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
> -				    data->vmperms, data->vmcount);
> +			err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
> +					data->vmperms, data->vmcount);
> +			if (err) {
> +				dev_err(rdev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
> +				    res.start, resource_size(&res), err);

Shouldn't the caller function report the error?

How as this found and tested?

thanks,

greg k-h
Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe
Posted by Bjorn Andersson 1 week, 5 days ago 
On Mon, Jan 26, 2026 at 04:24:55PM +0100, Greg KH wrote:
> On Sat, Jan 17, 2026 at 10:03:51PM +0800, Xingjing Deng wrote:
> > In the SDSP probe path, qcom_scm_assign_mem() is used to assign the
> > reserved memory to the configured VMIDs, but its return value was not
> > checked.
> > 
> > Fail the probe if the SCM call fails to avoid continuing with an
> > unexpected/incorrect memory permission configuration.
> > 
> > The file has passed the check of checkpatch.
> > 
> > Fixes: c3c0363bc72d4 ("misc: fastrpc: support complete DMA pool access to the DSP")
> > Cc: stable@vger.kernel.org # 6.11-rc1
> > Signed-off-by: Xingjing Deng <xjdeng@buaa.edu.cn>
> > ---
> > v5:
> > - Squash the functional change and indentation fix into a single patch.
> > - Link to v4: https://lore.kernel.org/linux-arm-msm/2026011637-statute-showy-2c3f@gregkh/T/#t
> > 
> > v4:
> > - Format the indentation
> > - Link to v3: https://lore.kernel.org/linux-arm-msm/20260113084352.72itrloj5w7qb5o3@hu-mojha-hyd.qualcomm.com/T/#t
> > 
> > v3:
> > - Add missing linux-kernel@vger.kernel.org to cc list.
> > - Standarlize changelog placement/format.
> > - Link to v2: https://lore.kernel.org/linux-arm-msm/20260113063618.e2ke47gy3hnfi67e@hu-mojha-hyd.qualcomm.com/T/#t
> > 
> > v2:
> > - Add Fixes: and Cc: stable tags.
> > - Link to v1: https://lore.kernel.org/linux-arm-msm/20260113022550.4029635-1-xjdeng@buaa.edu.cn/T/#u
> > ---
> >  drivers/misc/fastrpc.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > index fb3b54e05928..d9650efa443f 100644
> > --- a/drivers/misc/fastrpc.c
> > +++ b/drivers/misc/fastrpc.c
> > @@ -2338,8 +2338,13 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> >  		if (!err) {
> >  			src_perms = BIT(QCOM_SCM_VMID_HLOS);
> >  
> > -			qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
> > -				    data->vmperms, data->vmcount);
> > +			err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
> > +					data->vmperms, data->vmcount);
> > +			if (err) {
> > +				dev_err(rdev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
> > +				    res.start, resource_size(&res), err);
> 
> Shouldn't the caller function report the error?
> 

That is correct, all codepaths through qcom_scm_assign_mem() will either
be -ENOMEM or print an error message, so we shouldn't print yet another
message in the log here.

(The usefulness of the error message in qcom_scm_assign_mem() could
certainly be improved, but that's a separate matter/patch).

> How as this found and tested?
> 

Looking forward to Xingjing's answer here.

But failing to handle errors here means that we're ignoring the failure
to map memory to the DSP, which will fail us later. So, that part is
correct. Exiting through err_free_data looks good as well.

Regards,
Bjorn

> thanks,
> 
> greg k-h
>
Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe
Posted by Xingjing Deng 1 week, 5 days ago 
I identified this issue through static program analysis. All other
callers of this function validate its return value, so I believe a
validation check should also be added here.

Bjorn Andersson <andersson@kernel.org> 于2026年1月27日周二 04:53写道：
>
> On Mon, Jan 26, 2026 at 04:24:55PM +0100, Greg KH wrote:
> > On Sat, Jan 17, 2026 at 10:03:51PM +0800, Xingjing Deng wrote:
> > > In the SDSP probe path, qcom_scm_assign_mem() is used to assign the
> > > reserved memory to the configured VMIDs, but its return value was not
> > > checked.
> > >
> > > Fail the probe if the SCM call fails to avoid continuing with an
> > > unexpected/incorrect memory permission configuration.
> > >
> > > The file has passed the check of checkpatch.
> > >
> > > Fixes: c3c0363bc72d4 ("misc: fastrpc: support complete DMA pool access to the DSP")
> > > Cc: stable@vger.kernel.org # 6.11-rc1
> > > Signed-off-by: Xingjing Deng <xjdeng@buaa.edu.cn>
> > > ---
> > > v5:
> > > - Squash the functional change and indentation fix into a single patch.
> > > - Link to v4: https://lore.kernel.org/linux-arm-msm/2026011637-statute-showy-2c3f@gregkh/T/#t
> > >
> > > v4:
> > > - Format the indentation
> > > - Link to v3: https://lore.kernel.org/linux-arm-msm/20260113084352.72itrloj5w7qb5o3@hu-mojha-hyd.qualcomm.com/T/#t
> > >
> > > v3:
> > > - Add missing linux-kernel@vger.kernel.org to cc list.
> > > - Standarlize changelog placement/format.
> > > - Link to v2: https://lore.kernel.org/linux-arm-msm/20260113063618.e2ke47gy3hnfi67e@hu-mojha-hyd.qualcomm.com/T/#t
> > >
> > > v2:
> > > - Add Fixes: and Cc: stable tags.
> > > - Link to v1: https://lore.kernel.org/linux-arm-msm/20260113022550.4029635-1-xjdeng@buaa.edu.cn/T/#u
> > > ---
> > >  drivers/misc/fastrpc.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > > index fb3b54e05928..d9650efa443f 100644
> > > --- a/drivers/misc/fastrpc.c
> > > +++ b/drivers/misc/fastrpc.c
> > > @@ -2338,8 +2338,13 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> > >             if (!err) {
> > >                     src_perms = BIT(QCOM_SCM_VMID_HLOS);
> > >
> > > -                   qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
> > > -                               data->vmperms, data->vmcount);
> > > +                   err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
> > > +                                   data->vmperms, data->vmcount);
> > > +                   if (err) {
> > > +                           dev_err(rdev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
> > > +                               res.start, resource_size(&res), err);
> >
> > Shouldn't the caller function report the error?
> >
>
> That is correct, all codepaths through qcom_scm_assign_mem() will either
> be -ENOMEM or print an error message, so we shouldn't print yet another
> message in the log here.
>
> (The usefulness of the error message in qcom_scm_assign_mem() could
> certainly be improved, but that's a separate matter/patch).
>
> > How as this found and tested?
> >
>
> Looking forward to Xingjing's answer here.
>
> But failing to handle errors here means that we're ignoring the failure
> to map memory to the DSP, which will fail us later. So, that part is
> correct. Exiting through err_free_data looks good as well.
>
> Regards,
> Bjorn
>
> > thanks,
> >
> > greg k-h
> >
Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe
Posted by Greg KH 1 week, 4 days ago 
On Tue, Jan 27, 2026 at 10:18:38AM +0800, Xingjing Deng wrote:
> I identified this issue through static program analysis. All other
> callers of this function validate its return value, so I believe a
> validation check should also be added here.

Please don't top-post :(

Anyway, you MUST properly document the tools used to find issues like
this in your changelog text, as our rules require.  Please do so.

thanks,

greg k-h
Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe
Posted by Xingjing Deng 1 week, 4 days ago 
I understand the current situation. I need to record which static
analysis tool I used to identify this issue and clarify that no actual
testing was performed. However, I have a question: my static analysis
tool is not open-source, so how should I document this?

Greg KH <gregkh@linuxfoundation.org> 于2026年1月27日周二 15:10写道：
>
> On Tue, Jan 27, 2026 at 10:18:38AM +0800, Xingjing Deng wrote:
> > I identified this issue through static program analysis. All other
> > callers of this function validate its return value, so I believe a
> > validation check should also be added here.
>
> Please don't top-post :(
>
> Anyway, you MUST properly document the tools used to find issues like
> this in your changelog text, as our rules require.  Please do so.
>
> thanks,
>
> greg k-h
Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe
Posted by Xingjing Deng 1 week, 4 days ago 
I got it, I'll note that this is a private static analysis tool, and
I'll release the next version of the patch

Xingjing Deng <micro6947@gmail.com> 于2026年1月28日周三 10:29写道：
>
> I understand the current situation. I need to record which static
> analysis tool I used to identify this issue and clarify that no actual
> testing was performed. However, I have a question: my static analysis
> tool is not open-source, so how should I document this?
>
> Greg KH <gregkh@linuxfoundation.org> 于2026年1月27日周二 15:10写道：
> >
> > On Tue, Jan 27, 2026 at 10:18:38AM +0800, Xingjing Deng wrote:
> > > I identified this issue through static program analysis. All other
> > > callers of this function validate its return value, so I believe a
> > > validation check should also be added here.
> >
> > Please don't top-post :(
> >
> > Anyway, you MUST properly document the tools used to find issues like
> > this in your changelog text, as our rules require.  Please do so.
> >
> > thanks,
> >
> > greg k-h
Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe
Posted by Bjorn Andersson 1 week, 5 days ago 
On Tue, Jan 27, 2026 at 10:18:38AM +0800, Xingjing Deng wrote:
> I identified this issue through static program analysis. All other
> callers of this function validate its return value, so I believe a
> validation check should also be added here.
> 

I agree with your findings.

Please drop the dev_err() and mention that you found this through static
analysis in the commit message.

Thank you,
Bjorn

> Bjorn Andersson <andersson@kernel.org> 于2026年1月27日周二 04:53写道：
> >
> > On Mon, Jan 26, 2026 at 04:24:55PM +0100, Greg KH wrote:
> > > On Sat, Jan 17, 2026 at 10:03:51PM +0800, Xingjing Deng wrote:
> > > > In the SDSP probe path, qcom_scm_assign_mem() is used to assign the
> > > > reserved memory to the configured VMIDs, but its return value was not
> > > > checked.
> > > >
> > > > Fail the probe if the SCM call fails to avoid continuing with an
> > > > unexpected/incorrect memory permission configuration.
> > > >
> > > > The file has passed the check of checkpatch.
> > > >
> > > > Fixes: c3c0363bc72d4 ("misc: fastrpc: support complete DMA pool access to the DSP")
> > > > Cc: stable@vger.kernel.org # 6.11-rc1
> > > > Signed-off-by: Xingjing Deng <xjdeng@buaa.edu.cn>
> > > > ---
> > > > v5:
> > > > - Squash the functional change and indentation fix into a single patch.
> > > > - Link to v4: https://lore.kernel.org/linux-arm-msm/2026011637-statute-showy-2c3f@gregkh/T/#t
> > > >
> > > > v4:
> > > > - Format the indentation
> > > > - Link to v3: https://lore.kernel.org/linux-arm-msm/20260113084352.72itrloj5w7qb5o3@hu-mojha-hyd.qualcomm.com/T/#t
> > > >
> > > > v3:
> > > > - Add missing linux-kernel@vger.kernel.org to cc list.
> > > > - Standarlize changelog placement/format.
> > > > - Link to v2: https://lore.kernel.org/linux-arm-msm/20260113063618.e2ke47gy3hnfi67e@hu-mojha-hyd.qualcomm.com/T/#t
> > > >
> > > > v2:
> > > > - Add Fixes: and Cc: stable tags.
> > > > - Link to v1: https://lore.kernel.org/linux-arm-msm/20260113022550.4029635-1-xjdeng@buaa.edu.cn/T/#u
> > > > ---
> > > >  drivers/misc/fastrpc.c | 9 +++++++--
> > > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > > > index fb3b54e05928..d9650efa443f 100644
> > > > --- a/drivers/misc/fastrpc.c
> > > > +++ b/drivers/misc/fastrpc.c
> > > > @@ -2338,8 +2338,13 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> > > >             if (!err) {
> > > >                     src_perms = BIT(QCOM_SCM_VMID_HLOS);
> > > >
> > > > -                   qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
> > > > -                               data->vmperms, data->vmcount);
> > > > +                   err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
> > > > +                                   data->vmperms, data->vmcount);
> > > > +                   if (err) {
> > > > +                           dev_err(rdev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
> > > > +                               res.start, resource_size(&res), err);
> > >
> > > Shouldn't the caller function report the error?
> > >
> >
> > That is correct, all codepaths through qcom_scm_assign_mem() will either
> > be -ENOMEM or print an error message, so we shouldn't print yet another
> > message in the log here.
> >
> > (The usefulness of the error message in qcom_scm_assign_mem() could
> > certainly be improved, but that's a separate matter/patch).
> >
> > > How as this found and tested?
> > >
> >
> > Looking forward to Xingjing's answer here.
> >
> > But failing to handle errors here means that we're ignoring the failure
> > to map memory to the DSP, which will fail us later. So, that part is
> > correct. Exiting through err_free_data looks good as well.
> >
> > Regards,
> > Bjorn
> >
> > > thanks,
> > >
> > > greg k-h
> > >

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.