Handle bad policies for AMD-PMF better

From: Mario Limonciello <[email protected]>

There are assumptions in AMD-PMF code regarding policies being valid both from firmware and from sideloading. This series adds an extra test to ensure policies aren't empty and also handles a memory leak in the cleanup path for the sideloaded policies.

v2:
* call memchr_inv from helper
* Split out cleanup error to it's own patch
Mario Limonciello (2):
drivers/platform/x86/amd: pmf: Check for invalid sideloaded Smart PC
Policies
drivers/platform/x86/amd: pmf: Check for invalid Smart PC Policies

 drivers/platform/x86/amd/pmf/tee-if.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

2.43.0

From: Mario Limonciello <[email protected]>

If a policy is passed into amd_pmf_get_pb_data() that causes the engine to fail to start there is a memory leak. Free the memory in this failure path.

---
v2:
* Split to it's own patch

---
 drivers/platform/x86/amd/pmf/tee-if.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

index 4c49b510ad1a7..a8bf8c61669d3 100644
@@ -380,9 +380,14 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
 	amd_pmf_hex_dump_pb(dev);
 	ret = amd_pmf_start_policy_engine(dev);
 	if (ret < 0)
-		return ret;
+		goto cleanup;
 
 	return length;
+
+cleanup:
+	kfree(dev->policy_buf);
+	dev->policy_buf = NULL;
+	return ret;
 }
 
 static const struct file_operations pb_fops = {

2.43.0

From: Mario Limonciello <[email protected]>

commit 376a8c2a14439 ("platform/x86/amd/pmf: Update PMF Driver for Compatibility with new PMF-TA") added support for platforms that support an updated TA, however it also exposed a number of platforms that although they have support for the updated TA don't actually populate a policy binary.

Add an explicit check that the policy binary isn't empty before initializing the TA.

---
 drivers/platform/x86/amd/pmf/tee-if.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

index a8bf8c61669d3..7d6404ab9f041 100644
@@ -350,6 +350,11 @@ static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
 	return 0;
 }
 
+static inline bool amd_pmf_pb_valid(struct amd_pmf_dev *dev)
+{
+	return memchr_inv(dev->policy_buf, 0xff, dev->policy_sz);
+}
+
 #ifdef CONFIG_AMD_PMF_DEBUG
 static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev)
 {

@@ -377,6 +382,11 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
 	dev->policy_buf = new_policy_buf;
 	dev->policy_sz = length;
 
+	if (!amd_pmf_pb_valid(dev)) {
+		ret = -EINVAL;
+		goto cleanup;
+	}
+
 	amd_pmf_hex_dump_pb(dev);
 	ret = amd_pmf_start_policy_engine(dev);
 	if (ret < 0)

@@ -549,6 +559,12 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
 
 	memcpy_fromio(dev->policy_buf, dev->policy_base, dev->policy_sz);
 
+	if (!amd_pmf_pb_valid(dev)) {
+		dev_info(dev->dev, "No Smart PC policy present\n");
+		ret = -EINVAL;
+		goto err_free_policy;
+	}
+
 	amd_pmf_hex_dump_pb(dev);
 
 	dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);

2.43.0

Thanks for the quick fix!

Thank you for your contribution, it has been applied to my local review-ilpo-fixes branch. Note it will show up in the public platform-drivers-x86/review-ilpo-fixes branch only once I've pushed my local branch there, which might take a while.

The list of commits applied:
[1/2] drivers/platform/x86/amd: pmf: Check for invalid sideloaded Smart PC Policies

      commit: 690d722e02819ef978f90cd7553973eba1007e6c
[2/2] drivers/platform/x86/amd: pmf: Check for invalid Smart PC Policies
      commit: 8e81b9cd6e95188d12c9cc25d40b61dd5ea05ace

--
i.