* [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe
@ 2026-01-17 14:03 Xingjing Deng
2026-01-26 15:24 ` Greg KH
0 siblings, 1 reply; 8+ messages in thread
From: Xingjing Deng @ 2026-01-17 14:03 UTC (permalink / raw)
To: srini, amahesh, arnd, gregkh
Cc: dri-devel, linux-arm-msm, linux-kernel, Xingjing Deng, stable
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe 2026-01-17 14:03 [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe Xingjing Deng @ 2026-01-26 15:24 ` Greg KH 2026-01-26 20:53 ` Bjorn Andersson 0 siblings, 1 reply; 8+ messages in thread From: Greg KH @ 2026-01-26 15:24 UTC (permalink / raw) To: Xingjing Deng Cc: srini, amahesh, arnd, dri-devel, linux-arm-msm, linux-kernel, Xingjing Deng, stable 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe 2026-01-26 15:24 ` Greg KH @ 2026-01-26 20:53 ` Bjorn Andersson 2026-01-27 2:18 ` Xingjing Deng 0 siblings, 1 reply; 8+ messages in thread From: Bjorn Andersson @ 2026-01-26 20:53 UTC (permalink / raw) To: Greg KH Cc: Xingjing Deng, srini, amahesh, arnd, dri-devel, linux-arm-msm, linux-kernel, Xingjing Deng, stable 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe 2026-01-26 20:53 ` Bjorn Andersson @ 2026-01-27 2:18 ` Xingjing Deng 2026-01-27 2:46 ` Bjorn Andersson 2026-01-27 7:10 ` Greg KH 0 siblings, 2 replies; 8+ messages in thread From: Xingjing Deng @ 2026-01-27 2:18 UTC (permalink / raw) To: Bjorn Andersson Cc: Greg KH, srini, amahesh, arnd, dri-devel, linux-arm-msm, linux-kernel, Xingjing Deng, stable 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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe 2026-01-27 2:18 ` Xingjing Deng @ 2026-01-27 2:46 ` Bjorn Andersson 2026-01-27 7:10 ` Greg KH 1 sibling, 0 replies; 8+ messages in thread From: Bjorn Andersson @ 2026-01-27 2:46 UTC (permalink / raw) To: Xingjing Deng Cc: Greg KH, srini, amahesh, arnd, dri-devel, linux-arm-msm, linux-kernel, Xingjing Deng, stable 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 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe 2026-01-27 2:18 ` Xingjing Deng 2026-01-27 2:46 ` Bjorn Andersson @ 2026-01-27 7:10 ` Greg KH 2026-01-28 2:29 ` Xingjing Deng 1 sibling, 1 reply; 8+ messages in thread From: Greg KH @ 2026-01-27 7:10 UTC (permalink / raw) To: Xingjing Deng Cc: Bjorn Andersson, srini, amahesh, arnd, dri-devel, linux-arm-msm, linux-kernel, Xingjing Deng, stable 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe 2026-01-27 7:10 ` Greg KH @ 2026-01-28 2:29 ` Xingjing Deng 2026-01-28 2:34 ` Xingjing Deng 0 siblings, 1 reply; 8+ messages in thread From: Xingjing Deng @ 2026-01-28 2:29 UTC (permalink / raw) To: Greg KH Cc: Bjorn Andersson, srini, amahesh, arnd, dri-devel, linux-arm-msm, linux-kernel, Xingjing Deng, stable 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe 2026-01-28 2:29 ` Xingjing Deng @ 2026-01-28 2:34 ` Xingjing Deng 0 siblings, 0 replies; 8+ messages in thread From: Xingjing Deng @ 2026-01-28 2:34 UTC (permalink / raw) To: Greg KH Cc: Bjorn Andersson, srini, amahesh, arnd, dri-devel, linux-arm-msm, linux-kernel, Xingjing Deng, stable 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-01-28 2:34 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-01-17 14:03 [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in rpmsg_probe Xingjing Deng 2026-01-26 15:24 ` Greg KH 2026-01-26 20:53 ` Bjorn Andersson 2026-01-27 2:18 ` Xingjing Deng 2026-01-27 2:46 ` Bjorn Andersson 2026-01-27 7:10 ` Greg KH 2026-01-28 2:29 ` Xingjing Deng 2026-01-28 2:34 ` Xingjing Deng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox