Re: [PATCH V3 05/10] remoteproc: qcom_q6v5_mss: Use a carveout to authenticate modem headers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey Mani,

Thanks for taking time to review the series.

On 1/11/23 17:24, Manivannan Sadhasivam wrote:
On Wed, Jan 11, 2023 at 05:13:32PM +0530, Sibi Sankar wrote:
Any access to the dynamically allocated metadata 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-and-tested-by: Amit Pundir <amit.pundir@xxxxxxxxxx>
Fixes: 6c5a9dc2481b ("remoteproc: qcom: Make secure world call for mem ownership switch")
Signed-off-by: Sibi Sankar <quic_sibis@xxxxxxxxxxx>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>

---

v3:
  * Drop revert no_kernel_mapping since it's already on the list [Mani]

I thought you are going to include Christoph's patch into your series. That way
all the patches will be in the same series, makig life easier for Bjorn.


Since there were multiple patches in Christoph's original series, I
decided I'll just drop the revert and have it depends on instead.


Thanks,
Mani

  * kfree metadata from the branch for parity

  drivers/remoteproc/qcom_q6v5_mss.c | 48 ++++++++++++++++++++++++++----
  1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index e2f765f87ec9..e25d44e20ae7 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -215,6 +215,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;
@@ -973,15 +974,29 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw,
  	if (IS_ERR(metadata))
  		return PTR_ERR(metadata);
- ptr = dma_alloc_attrs(qproc->dev, size, &phys, GFP_KERNEL, dma_attrs);
-	if (!ptr) {
-		kfree(metadata);
-		dev_err(qproc->dev, "failed to allocate mdt buffer\n");
-		return -ENOMEM;
+	if (qproc->mdata_phys) {
+		phys = qproc->mdata_phys;
+		ptr = memremap(qproc->mdata_phys, size, MEMREMAP_WC);
+		if (!ptr) {
+			kfree(metadata);
+			dev_err(qproc->dev, "unable to map memory region: %pa+%zx\n",
+				&qproc->mdata_phys, size);
+			return -EBUSY;
+		}
+	} else {
+		ptr = dma_alloc_attrs(qproc->dev, size, &phys, GFP_KERNEL, dma_attrs);
+		if (!ptr) {
+			kfree(metadata);
+			dev_err(qproc->dev, "failed to allocate mdt buffer\n");
+			return -ENOMEM;
+		}
  	}
memcpy(ptr, metadata, size); + if (qproc->mdata_phys)
+		memunmap(ptr);
+
  	/* Hypervisor mapping to access metadata by modem */
  	mdata_perm = BIT(QCOM_SCM_VMID_HLOS);
  	ret = q6v5_xfer_mem_ownership(qproc, &mdata_perm, false, true,
@@ -1010,7 +1025,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, ptr, phys, dma_attrs);
+	if (!qproc->mdata_phys)
+		dma_free_attrs(qproc->dev, size, ptr, phys, dma_attrs);
  	kfree(metadata);
return ret < 0 ? ret : 0;
@@ -1893,6 +1909,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





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux