[PATCH v2] platform/x86/amd/pmf: Fix ACPI buffer validation in APTS path

[PATCH v2] platform/x86/amd/pmf: Fix ACPI buffer validation in APTS path

[Shyam Sundar S K]

apts_if_call_store_buffer() reads a u16 size from info->buffer.pointer
without checking that it is non-NULL and at least 2 bytes long, so a
short or NULL firmware buffer can cause an out-of-bounds read.
apmf_if_call_store_buffer() checked the length but not the pointer.

Add a helper, amd_pmf_if_verify_buffer(), that rejects a NULL pointer or
a buffer smaller than the 2-byte header, and call it from both paths, so
the check is done consistently.

Fixes: 3eecb434d7f2 ("platform/x86/amd/pmf: Add support to get sps default APTS index values")
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
v2:
 - Add a helper function for NULL pointer and buffer validation. Reuse it in
   apmf_if_call_store_buffer() and apts_if_call_store_buffer().

 drivers/platform/x86/amd/pmf/acpi.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
index 3d94b03cf794..efad01a4ed27 100644
--- a/drivers/platform/x86/amd/pmf/acpi.c
+++ b/drivers/platform/x86/amd/pmf/acpi.c
@@ -50,6 +50,16 @@ static union acpi_object *apmf_if_call(struct amd_pmf_dev *pdev, int fn, struct
 	return buffer.pointer;
 }
 
+static int amd_pmf_if_verify_buffer(struct amd_pmf_dev *pdev, union acpi_object *info)
+{
+	if (!info->buffer.pointer || info->buffer.length < 2) {
+		dev_err(pdev->dev, "buffer pointer is NULL or too small\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int apmf_if_call_store_buffer(struct amd_pmf_dev *pdev, int fn, void *dest, size_t out_sz)
 {
 	union acpi_object *info;
@@ -66,11 +76,9 @@ static int apmf_if_call_store_buffer(struct amd_pmf_dev *pdev, int fn, void *des
 		goto out;
 	}
 
-	if (info->buffer.length < 2) {
-		dev_err(pdev->dev, "buffer too small\n");
-		err = -EINVAL;
+	err = amd_pmf_if_verify_buffer(pdev, info);
+	if (err)
 		goto out;
-	}
 
 	size = *(u16 *)info->buffer.pointer;
 	if (info->buffer.length < size) {
@@ -138,6 +146,10 @@ static int apts_if_call_store_buffer(struct amd_pmf_dev *pdev,
 		goto out;
 	}
 
+	err = amd_pmf_if_verify_buffer(pdev, info);
+	if (err)
+		goto out;
+
 	size = *(u16 *)info->buffer.pointer;
 	if (info->buffer.length < size) {
 		dev_err(pdev->dev, "buffer smaller than header size %u < %zu\n",
-- 
2.34.1

Re: [PATCH v2] platform/x86/amd/pmf: Fix ACPI buffer validation in APTS path

[Mario Limonciello]

On 9/30/26 12:24, Shyam Sundar S K wrote:
> apts_if_call_store_buffer() reads a u16 size from info->buffer.pointer
> without checking that it is non-NULL and at least 2 bytes long, so a
> short or NULL firmware buffer can cause an out-of-bounds read.
> apmf_if_call_store_buffer() checked the length but not the pointer.
> 
> Add a helper, amd_pmf_if_verify_buffer(), that rejects a NULL pointer or
> a buffer smaller than the 2-byte header, and call it from both paths, so
> the check is done consistently.
> 
> Fixes: 3eecb434d7f2 ("platform/x86/amd/pmf: Add support to get sps default APTS index values")
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> v2:
>   - Add a helper function for NULL pointer and buffer validation. Reuse it in
>     apmf_if_call_store_buffer() and apts_if_call_store_buffer().
> 
>   drivers/platform/x86/amd/pmf/acpi.c | 20 ++++++++++++++++----
>   1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index 3d94b03cf794..efad01a4ed27 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -50,6 +50,16 @@ static union acpi_object *apmf_if_call(struct amd_pmf_dev *pdev, int fn, struct
>   	return buffer.pointer;
>   }
>   
> +static int amd_pmf_if_verify_buffer(struct amd_pmf_dev *pdev, union acpi_object *info)
> +{
> +	if (!info->buffer.pointer || info->buffer.length < 2) {
> +		dev_err(pdev->dev, "buffer pointer is NULL or too small\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>   static int apmf_if_call_store_buffer(struct amd_pmf_dev *pdev, int fn, void *dest, size_t out_sz)
>   {
>   	union acpi_object *info;
> @@ -66,11 +76,9 @@ static int apmf_if_call_store_buffer(struct amd_pmf_dev *pdev, int fn, void *des
>   		goto out;
>   	}
>   
> -	if (info->buffer.length < 2) {
> -		dev_err(pdev->dev, "buffer too small\n");
> -		err = -EINVAL;
> +	err = amd_pmf_if_verify_buffer(pdev, info);
> +	if (err)
>   		goto out;
> -	}
>   
>   	size = *(u16 *)info->buffer.pointer;
>   	if (info->buffer.length < size) {
> @@ -138,6 +146,10 @@ static int apts_if_call_store_buffer(struct amd_pmf_dev *pdev,
>   		goto out;
>   	}
>   
> +	err = amd_pmf_if_verify_buffer(pdev, info);
> +	if (err)
> +		goto out;
> +
>   	size = *(u16 *)info->buffer.pointer;
>   	if (info->buffer.length < size) {
>   		dev_err(pdev->dev, "buffer smaller than header size %u < %zu\n",


If I'm not mistaken you can actually fold in more to the common check.

if (info->type != ACPI_TYPE_BUFFER) {

and buffer size check

if (info->buffer.length < size) {

and then

	if (size < out_sz) {

Then you could just return the size on success from the helper both

apts_if_call_store_buffer calls
1) apts_if_call
2) helper
3) memcpy
4) kfree