| 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
| ||
|
Message-ID: <20221213140724.8612-5-quic_sibis@quicinc.com>
Date: Tue, 13 Dec 2022 19:37:24 +0530
From: Sibi Sankar <quic_sibis@...cinc.com>
To: <andersson@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
<robh+dt@...nel.org>, <manivannan.sadhasivam@...aro.org>
CC: <agross@...nel.org>, <linux-arm-msm@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<konrad.dybcio@...ainline.org>, <amit.pundir@...aro.org>,
<regressions@...mhuis.info>, <sumit.semwal@...aro.org>,
<will@...nel.org>, <catalin.marinas@....com>,
<robin.murphy@....com>, Sibi Sankar <quic_sibis@...cinc.com>
Subject: [PATCH 4/4] remoteproc: qcom_q6v5_mss: Use a carveout to authenticate modem headers
The memory region allocated using dma_alloc_attr with no kernel mapping
attribute set would still be a part of the linear kernel map. Any access
to this region by the application processor after assigning it to the
remote Q6 will result in a XPU violation. Fix this by replacing the
dynamically allocated memory region with a no-map carveout and unmap the
modem metadata memory region before passing control to the remote Q6.
Reported-by: Amit Pundir <amit.pundir@...aro.org>
Fixes: 6c5a9dc2481b ("remoteproc: qcom: Make secure world call for mem ownership switch")
Signed-off-by: Sibi Sankar <quic_sibis@...cinc.com>
---
The addition of the carveout and memunmap is required only on SoCs that
mandate memory protection before transferring control to Q6, hence the
driver falls back to dynamic memory allocation in the absence of the
modem metadata carveout.
drivers/remoteproc/qcom_q6v5_mss.c | 85 +++++++++++++++++++++---------
1 file changed, 61 insertions(+), 24 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index fddb63cffee0..8264275ecbd0 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -211,6 +211,7 @@ struct q6v5 {
size_t mba_size;
size_t dp_size;
+ phys_addr_t mdata_phys;
phys_addr_t mpss_phys;
phys_addr_t mpss_reloc;
size_t mpss_size;
@@ -935,6 +936,7 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw,
{
unsigned long dma_attrs = DMA_ATTR_FORCE_CONTIGUOUS | DMA_ATTR_NO_KERNEL_MAPPING;
unsigned long flags = VM_DMA_COHERENT | VM_FLUSH_RESET_PERMS;
+ void *mdata_region;
struct page **pages;
struct page *page;
dma_addr_t phys;
@@ -951,34 +953,48 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw,
if (IS_ERR(metadata))
return PTR_ERR(metadata);
- page = dma_alloc_attrs(qproc->dev, size, &phys, GFP_KERNEL, dma_attrs);
- if (!page) {
- kfree(metadata);
- dev_err(qproc->dev, "failed to allocate mdt buffer\n");
- return -ENOMEM;
- }
+ if (qproc->mdata_phys) {
+ mdata_region = memremap(qproc->mdata_phys, size, MEMREMAP_WC);
+ if (!mdata_region) {
+ dev_err(qproc->dev, "unable to map memory region: %pa+%zx\n",
+ &qproc->mdata_phys, size);
+ ret = -EBUSY;
+ goto free_dma_attrs;
+ }
- count = PAGE_ALIGN(size) >> PAGE_SHIFT;
- pages = kmalloc_array(count, sizeof(struct page *), GFP_KERNEL);
- if (!pages) {
- ret = -ENOMEM;
- goto free_dma_attrs;
- }
+ memcpy(mdata_region, metadata, size);
+ memunmap(mdata_region);
+ phys = qproc->mdata_phys;
+ } else {
+ page = dma_alloc_attrs(qproc->dev, size, &phys, GFP_KERNEL, dma_attrs);
+ if (!page) {
+ kfree(metadata);
+ dev_err(qproc->dev, "failed to allocate mdt buffer\n");
+ return -ENOMEM;
+ }
- for (i = 0; i < count; i++)
- pages[i] = nth_page(page, i);
+ count = PAGE_ALIGN(size) >> PAGE_SHIFT;
+ pages = kmalloc_array(count, sizeof(struct page *), GFP_KERNEL);
+ if (!pages) {
+ ret = -ENOMEM;
+ goto free_dma_attrs;
+ }
- vaddr = vmap(pages, count, flags, pgprot_dmacoherent(PAGE_KERNEL));
- kfree(pages);
- if (!vaddr) {
- dev_err(qproc->dev, "unable to map memory region: %pa+%zx\n", &phys, size);
- ret = -EBUSY;
- goto free_dma_attrs;
- }
+ for (i = 0; i < count; i++)
+ pages[i] = nth_page(page, i);
- memcpy(vaddr, metadata, size);
+ vaddr = vmap(pages, count, flags, pgprot_dmacoherent(PAGE_KERNEL));
+ kfree(pages);
+ if (!vaddr) {
+ dev_err(qproc->dev, "unable to map memory region: %pa+%zx\n", &phys, size);
+ ret = -EBUSY;
+ goto free_dma_attrs;
+ }
- vunmap(vaddr);
+ memcpy(vaddr, metadata, size);
+
+ vunmap(vaddr);
+ }
/* Hypervisor mapping to access metadata by modem */
mdata_perm = BIT(QCOM_SCM_VMID_HLOS);
@@ -1008,7 +1024,8 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw,
"mdt buffer not reclaimed system may become unstable\n");
free_dma_attrs:
- dma_free_attrs(qproc->dev, size, page, phys, dma_attrs);
+ if (!qproc->mdata_phys)
+ dma_free_attrs(qproc->dev, size, page, phys, dma_attrs);
kfree(metadata);
return ret < 0 ? ret : 0;
@@ -1882,6 +1899,26 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)
qproc->mpss_phys = qproc->mpss_reloc = r.start;
qproc->mpss_size = resource_size(&r);
+ if (!child) {
+ node = of_parse_phandle(qproc->dev->of_node, "memory-region", 2);
+ } else {
+ child = of_get_child_by_name(qproc->dev->of_node, "metadata");
+ node = of_parse_phandle(child, "memory-region", 0);
+ of_node_put(child);
+ }
+
+ if (!node)
+ return 0;
+
+ ret = of_address_to_resource(node, 0, &r);
+ of_node_put(node);
+ if (ret) {
+ dev_err(qproc->dev, "unable to resolve metadata region\n");
+ return ret;
+ }
+
+ qproc->mdata_phys = r.start;
+
return 0;
}
--
2.17.1
Powered by blists - more mailing lists