* [PATCH v3] platform/x86/amd/pmf: Fix ACPI buffer validation in APTS path
@ 2026-10-05 7:28 Shyam Sundar S K
2026-10-05 14:03 ` Mario Limonciello
0 siblings, 1 reply; 2+ messages in thread
From: Shyam Sundar S K @ 2026-10-05 7:28 UTC (permalink / raw)
To: hansg, ilpo.jarvinen
Cc: platform-driver-x86, Patil.Reddy, mario.limonciello,
Shyam Sundar S K
apts_if_call_store_buffer() reads a u16 size from info->buffer.pointer
without checking it is non-NULL and at least 2 bytes long, so a short or
NULL firmware buffer can cause an out-of-bounds read.
Add a common helper, amd_pmf_if_verify_buffer(), that validates the
buffer (type, NULL or short pointer, header and output size) and returns
the size on success. Call it from both apts_if_call_store_buffer() and
apmf_if_call_store_buffer().
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>
---
v3:
- Move full buffer validation (type, NULL or short pointer, header and
output size checks) into verify buffer helper.
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 | 70 +++++++++++++----------------
1 file changed, 30 insertions(+), 40 deletions(-)
diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
index 3d94b03cf794..59173bcc3750 100644
--- a/drivers/platform/x86/amd/pmf/acpi.c
+++ b/drivers/platform/x86/amd/pmf/acpi.c
@@ -50,47 +50,54 @@ static union acpi_object *apmf_if_call(struct amd_pmf_dev *pdev, int fn, struct
return buffer.pointer;
}
-static int apmf_if_call_store_buffer(struct amd_pmf_dev *pdev, int fn, void *dest, size_t out_sz)
+static int amd_pmf_if_verify_buffer(struct amd_pmf_dev *pdev,
+ union acpi_object *info, size_t out_sz)
{
- union acpi_object *info;
size_t size;
- int err = 0;
-
- info = apmf_if_call(pdev, fn, NULL);
- if (!info)
- return -EIO;
if (info->type != ACPI_TYPE_BUFFER) {
dev_err(pdev->dev, "object is not a buffer\n");
- err = -EINVAL;
- goto out;
+ return -EINVAL;
}
- if (info->buffer.length < 2) {
- dev_err(pdev->dev, "buffer too small\n");
- err = -EINVAL;
- goto out;
+ if (!info->buffer.pointer || info->buffer.length < 2) {
+ dev_err(pdev->dev, "buffer pointer is NULL or too small\n");
+ return -EINVAL;
}
size = *(u16 *)info->buffer.pointer;
if (info->buffer.length < size) {
- dev_err(pdev->dev, "buffer smaller then headersize %u < %zu\n",
+ dev_err(pdev->dev, "buffer smaller than headersize %u < %zu\n",
info->buffer.length, size);
- err = -EINVAL;
- goto out;
+ return -EINVAL;
}
if (size < out_sz) {
dev_err(pdev->dev, "buffer too small %zu\n", size);
- err = -EINVAL;
- goto out;
+ return -EINVAL;
}
+ return (int)size;
+}
+
+static int apmf_if_call_store_buffer(struct amd_pmf_dev *pdev, int fn, void *dest, size_t out_sz)
+{
+ union acpi_object *info;
+ int size;
+
+ info = apmf_if_call(pdev, fn, NULL);
+ if (!info)
+ return -EIO;
+
+ size = amd_pmf_if_verify_buffer(pdev, info, out_sz);
+ if (size < 0)
+ goto out;
+
memcpy(dest, info->buffer.pointer, out_sz);
out:
kfree(info);
- return err;
+ return size < 0 ? size : 0;
}
static union acpi_object *apts_if_call(struct amd_pmf_dev *pdev, u32 state_index)
@@ -125,37 +132,20 @@ static int apts_if_call_store_buffer(struct amd_pmf_dev *pdev,
u32 index, void *data, size_t out_sz)
{
union acpi_object *info;
- size_t size;
- int err = 0;
+ int size;
info = apts_if_call(pdev, index);
if (!info)
return -EIO;
- if (info->type != ACPI_TYPE_BUFFER) {
- dev_err(pdev->dev, "object is not a buffer\n");
- err = -EINVAL;
- goto out;
- }
-
- size = *(u16 *)info->buffer.pointer;
- if (info->buffer.length < size) {
- dev_err(pdev->dev, "buffer smaller than header size %u < %zu\n",
- info->buffer.length, size);
- err = -EINVAL;
- goto out;
- }
-
- if (size < out_sz) {
- dev_err(pdev->dev, "buffer too small %zu\n", size);
- err = -EINVAL;
+ size = amd_pmf_if_verify_buffer(pdev, info, out_sz);
+ if (size < 0)
goto out;
- }
memcpy(data, info->buffer.pointer, out_sz);
out:
kfree(info);
- return err;
+ return size < 0 ? size : 0;
}
int is_apmf_func_supported(struct amd_pmf_dev *pdev, unsigned long index)
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] platform/x86/amd/pmf: Fix ACPI buffer validation in APTS path
2026-10-05 7:28 [PATCH v3] platform/x86/amd/pmf: Fix ACPI buffer validation in APTS path Shyam Sundar S K
@ 2026-10-05 14:03 ` Mario Limonciello
0 siblings, 0 replies; 2+ messages in thread
From: Mario Limonciello @ 2026-10-05 14:03 UTC (permalink / raw)
To: Shyam Sundar S K, hansg, ilpo.jarvinen; +Cc: platform-driver-x86, Patil.Reddy
On 10/5/26 02:28, Shyam Sundar S K wrote:
> apts_if_call_store_buffer() reads a u16 size from info->buffer.pointer
> without checking it is non-NULL and at least 2 bytes long, so a short or
> NULL firmware buffer can cause an out-of-bounds read.
>
> Add a common helper, amd_pmf_if_verify_buffer(), that validates the
> buffer (type, NULL or short pointer, header and output size) and returns
> the size on success. Call it from both apts_if_call_store_buffer() and
> apmf_if_call_store_buffer().
>
> 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>
Thanks for following my suggestions.
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> v3:
> - Move full buffer validation (type, NULL or short pointer, header and
> output size checks) into verify buffer helper.
>
> 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 | 70 +++++++++++++----------------
> 1 file changed, 30 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index 3d94b03cf794..59173bcc3750 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -50,47 +50,54 @@ static union acpi_object *apmf_if_call(struct amd_pmf_dev *pdev, int fn, struct
> return buffer.pointer;
> }
>
> -static int apmf_if_call_store_buffer(struct amd_pmf_dev *pdev, int fn, void *dest, size_t out_sz)
> +static int amd_pmf_if_verify_buffer(struct amd_pmf_dev *pdev,
> + union acpi_object *info, size_t out_sz)
> {
> - union acpi_object *info;
> size_t size;
> - int err = 0;
> -
> - info = apmf_if_call(pdev, fn, NULL);
> - if (!info)
> - return -EIO;
>
> if (info->type != ACPI_TYPE_BUFFER) {
> dev_err(pdev->dev, "object is not a buffer\n");
> - err = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
>
> - if (info->buffer.length < 2) {
> - dev_err(pdev->dev, "buffer too small\n");
> - err = -EINVAL;
> - goto out;
> + if (!info->buffer.pointer || info->buffer.length < 2) {
> + dev_err(pdev->dev, "buffer pointer is NULL or too small\n");
> + return -EINVAL;
> }
>
> size = *(u16 *)info->buffer.pointer;
> if (info->buffer.length < size) {
> - dev_err(pdev->dev, "buffer smaller then headersize %u < %zu\n",
> + dev_err(pdev->dev, "buffer smaller than headersize %u < %zu\n",
> info->buffer.length, size);
> - err = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
>
> if (size < out_sz) {
> dev_err(pdev->dev, "buffer too small %zu\n", size);
> - err = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
>
> + return (int)size;
> +}
> +
> +static int apmf_if_call_store_buffer(struct amd_pmf_dev *pdev, int fn, void *dest, size_t out_sz)
> +{
> + union acpi_object *info;
> + int size;
> +
> + info = apmf_if_call(pdev, fn, NULL);
> + if (!info)
> + return -EIO;
> +
> + size = amd_pmf_if_verify_buffer(pdev, info, out_sz);
> + if (size < 0)
> + goto out;
> +
> memcpy(dest, info->buffer.pointer, out_sz);
>
> out:
> kfree(info);
> - return err;
> + return size < 0 ? size : 0;
> }
>
> static union acpi_object *apts_if_call(struct amd_pmf_dev *pdev, u32 state_index)
> @@ -125,37 +132,20 @@ static int apts_if_call_store_buffer(struct amd_pmf_dev *pdev,
> u32 index, void *data, size_t out_sz)
> {
> union acpi_object *info;
> - size_t size;
> - int err = 0;
> + int size;
>
> info = apts_if_call(pdev, index);
> if (!info)
> return -EIO;
>
> - if (info->type != ACPI_TYPE_BUFFER) {
> - dev_err(pdev->dev, "object is not a buffer\n");
> - err = -EINVAL;
> - goto out;
> - }
> -
> - size = *(u16 *)info->buffer.pointer;
> - if (info->buffer.length < size) {
> - dev_err(pdev->dev, "buffer smaller than header size %u < %zu\n",
> - info->buffer.length, size);
> - err = -EINVAL;
> - goto out;
> - }
> -
> - if (size < out_sz) {
> - dev_err(pdev->dev, "buffer too small %zu\n", size);
> - err = -EINVAL;
> + size = amd_pmf_if_verify_buffer(pdev, info, out_sz);
> + if (size < 0)
> goto out;
> - }
>
> memcpy(data, info->buffer.pointer, out_sz);
> out:
> kfree(info);
> - return err;
> + return size < 0 ? size : 0;
> }
>
> int is_apmf_func_supported(struct amd_pmf_dev *pdev, unsigned long index)
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-05 14:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-05 7:28 [PATCH v3] platform/x86/amd/pmf: Fix ACPI buffer validation in APTS path Shyam Sundar S K
2026-10-05 14:03 ` Mario Limonciello
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.