* [PATCH] ACPI: APEI: EINJ: Fix EINJV2 initialization and injection
@ 2025-11-19 1:27 Tony Luck
2025-11-19 12:34 ` Rafael J. Wysocki
0 siblings, 1 reply; 2+ messages in thread
From: Tony Luck @ 2025-11-19 1:27 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: bp, guohanjun, lenb, mchehab, xueshuai, yi1.lai, zaidal,
linux-acpi, linux-kernel, patches, Tony Luck
ACPI 6.6 specification for EINJV2 appends an extra structure to
the end of the existing struct set_error_type_with_address.
Several issues showed up in testing.
1) Initialization was broken by an earlier fix[1] since is_v2 is only set
while performing an injection, not during initialization.
2) A buggy BIOS provided invalid "revision" and "length" for the extension
structure. Add several sanity checks.
3) When injecting legacy error types on an EINJV2 capable system don't
copy the component arrays.
Fixes: 6c7058514991 ("ACPI: APEI: EINJ: Check if user asked for EINJV2 injection") # [1]
Fixes: b47610296d17 ("ACPI: APEI: EINJ: Enable EINJv2 error injections")
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
drivers/acpi/apei/einj-core.c | 64 ++++++++++++++++++++++-------------
1 file changed, 41 insertions(+), 23 deletions(-)
diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index 3c87953dbd19..305c240a303f 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -182,6 +182,7 @@ bool einj_initialized __ro_after_init;
static void __iomem *einj_param;
static u32 v5param_size;
+static u32 v66param_size;
static bool is_v2;
static void einj_exec_ctx_init(struct apei_exec_context *ctx)
@@ -283,6 +284,24 @@ static void check_vendor_extension(u64 paddr,
acpi_os_unmap_iomem(p, sizeof(v));
}
+static u32 einjv2_init(struct einjv2_extension_struct *e)
+{
+ if (e->revision != 1) {
+ pr_info("Unknown v2 extension revision %u\n", e->revision);
+ return 0;
+ }
+ if (e->length < sizeof(*e) || e->length > PAGE_SIZE) {
+ pr_info(FW_BUG "Bad1 v2 extension length %u\n", e->length);
+ return 0;
+ }
+ if ((e->length - sizeof(*e)) % sizeof(e->component_arr[0])) {
+ pr_info(FW_BUG "Bad2 v2 extension length %u\n", e->length);
+ return 0;
+ }
+
+ return (e->length - sizeof(*e)) / sizeof(e->component_arr[0]);
+}
+
static void __iomem *einj_get_parameter_address(void)
{
int i;
@@ -310,28 +329,21 @@ static void __iomem *einj_get_parameter_address(void)
v5param_size = sizeof(v5param);
p = acpi_os_map_iomem(pa_v5, sizeof(*p));
if (p) {
- int offset, len;
-
memcpy_fromio(&v5param, p, v5param_size);
acpi5 = 1;
check_vendor_extension(pa_v5, &v5param);
- if (is_v2 && available_error_type & ACPI65_EINJV2_SUPP) {
- len = v5param.einjv2_struct.length;
- offset = offsetof(struct einjv2_extension_struct, component_arr);
- max_nr_components = (len - offset) /
- sizeof(v5param.einjv2_struct.component_arr[0]);
- /*
- * The first call to acpi_os_map_iomem above does not include the
- * component array, instead it is used to read and calculate maximum
- * number of components supported by the system. Below, the mapping
- * is expanded to include the component array.
- */
+ if (available_error_type & ACPI65_EINJV2_SUPP) {
+ struct einjv2_extension_struct *e;
+
+ e = &v5param.einjv2_struct;
+ max_nr_components = einjv2_init(e);
+
+ /* remap including einjv2_extension_struct */
acpi_os_unmap_iomem(p, v5param_size);
- offset = offsetof(struct set_error_type_with_address, einjv2_struct);
- v5param_size = offset + struct_size(&v5param.einjv2_struct,
- component_arr, max_nr_components);
- p = acpi_os_map_iomem(pa_v5, v5param_size);
+ v66param_size = v5param_size - sizeof(*e) + e->length;
+ p = acpi_os_map_iomem(pa_v5, v66param_size);
}
+
return p;
}
}
@@ -527,6 +539,7 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
u64 param3, u64 param4)
{
struct apei_exec_context ctx;
+ u32 param_size = is_v2 ? v66param_size : v5param_size;
u64 val, trigger_paddr, timeout = FIRMWARE_TIMEOUT;
int i, rc;
@@ -539,11 +552,11 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
if (acpi5) {
struct set_error_type_with_address *v5param;
- v5param = kmalloc(v5param_size, GFP_KERNEL);
+ v5param = kmalloc(param_size, GFP_KERNEL);
if (!v5param)
return -ENOMEM;
- memcpy_fromio(v5param, einj_param, v5param_size);
+ memcpy_fromio(v5param, einj_param, param_size);
v5param->type = type;
if (type & ACPI5_VENDOR_BIT) {
switch (vendor_flags) {
@@ -601,7 +614,7 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
break;
}
}
- memcpy_toio(einj_param, v5param, v5param_size);
+ memcpy_toio(einj_param, v5param, param_size);
kfree(v5param);
} else {
rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
@@ -1132,9 +1145,14 @@ static void einj_remove(struct faux_device *fdev)
struct apei_exec_context ctx;
if (einj_param) {
- acpi_size size = (acpi5) ?
- v5param_size :
- sizeof(struct einj_parameter);
+ acpi_size size;
+
+ if (v66param_size)
+ size = v66param_size;
+ else if (acpi5)
+ size = v5param_size;
+ else
+ size = sizeof(struct einj_parameter);
acpi_os_unmap_iomem(einj_param, size);
if (vendor_errors.size)
--
2.51.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] ACPI: APEI: EINJ: Fix EINJV2 initialization and injection
2025-11-19 1:27 [PATCH] ACPI: APEI: EINJ: Fix EINJV2 initialization and injection Tony Luck
@ 2025-11-19 12:34 ` Rafael J. Wysocki
0 siblings, 0 replies; 2+ messages in thread
From: Rafael J. Wysocki @ 2025-11-19 12:34 UTC (permalink / raw)
To: Tony Luck
Cc: Rafael J. Wysocki, bp, guohanjun, lenb, mchehab, xueshuai,
yi1.lai, zaidal, linux-acpi, linux-kernel, patches
On Wed, Nov 19, 2025 at 2:27 AM Tony Luck <tony.luck@intel.com> wrote:
>
> ACPI 6.6 specification for EINJV2 appends an extra structure to
> the end of the existing struct set_error_type_with_address.
>
> Several issues showed up in testing.
>
> 1) Initialization was broken by an earlier fix[1] since is_v2 is only set
> while performing an injection, not during initialization.
>
> 2) A buggy BIOS provided invalid "revision" and "length" for the extension
> structure. Add several sanity checks.
>
> 3) When injecting legacy error types on an EINJV2 capable system don't
> copy the component arrays.
>
> Fixes: 6c7058514991 ("ACPI: APEI: EINJ: Check if user asked for EINJV2 injection") # [1]
> Fixes: b47610296d17 ("ACPI: APEI: EINJ: Enable EINJv2 error injections")
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
> drivers/acpi/apei/einj-core.c | 64 ++++++++++++++++++++++-------------
> 1 file changed, 41 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index 3c87953dbd19..305c240a303f 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -182,6 +182,7 @@ bool einj_initialized __ro_after_init;
>
> static void __iomem *einj_param;
> static u32 v5param_size;
> +static u32 v66param_size;
> static bool is_v2;
>
> static void einj_exec_ctx_init(struct apei_exec_context *ctx)
> @@ -283,6 +284,24 @@ static void check_vendor_extension(u64 paddr,
> acpi_os_unmap_iomem(p, sizeof(v));
> }
>
> +static u32 einjv2_init(struct einjv2_extension_struct *e)
> +{
> + if (e->revision != 1) {
> + pr_info("Unknown v2 extension revision %u\n", e->revision);
> + return 0;
> + }
> + if (e->length < sizeof(*e) || e->length > PAGE_SIZE) {
> + pr_info(FW_BUG "Bad1 v2 extension length %u\n", e->length);
> + return 0;
> + }
> + if ((e->length - sizeof(*e)) % sizeof(e->component_arr[0])) {
> + pr_info(FW_BUG "Bad2 v2 extension length %u\n", e->length);
> + return 0;
> + }
> +
> + return (e->length - sizeof(*e)) / sizeof(e->component_arr[0]);
> +}
> +
> static void __iomem *einj_get_parameter_address(void)
> {
> int i;
> @@ -310,28 +329,21 @@ static void __iomem *einj_get_parameter_address(void)
> v5param_size = sizeof(v5param);
> p = acpi_os_map_iomem(pa_v5, sizeof(*p));
> if (p) {
> - int offset, len;
> -
> memcpy_fromio(&v5param, p, v5param_size);
> acpi5 = 1;
> check_vendor_extension(pa_v5, &v5param);
> - if (is_v2 && available_error_type & ACPI65_EINJV2_SUPP) {
> - len = v5param.einjv2_struct.length;
> - offset = offsetof(struct einjv2_extension_struct, component_arr);
> - max_nr_components = (len - offset) /
> - sizeof(v5param.einjv2_struct.component_arr[0]);
> - /*
> - * The first call to acpi_os_map_iomem above does not include the
> - * component array, instead it is used to read and calculate maximum
> - * number of components supported by the system. Below, the mapping
> - * is expanded to include the component array.
> - */
> + if (available_error_type & ACPI65_EINJV2_SUPP) {
> + struct einjv2_extension_struct *e;
> +
> + e = &v5param.einjv2_struct;
> + max_nr_components = einjv2_init(e);
> +
> + /* remap including einjv2_extension_struct */
> acpi_os_unmap_iomem(p, v5param_size);
> - offset = offsetof(struct set_error_type_with_address, einjv2_struct);
> - v5param_size = offset + struct_size(&v5param.einjv2_struct,
> - component_arr, max_nr_components);
> - p = acpi_os_map_iomem(pa_v5, v5param_size);
> + v66param_size = v5param_size - sizeof(*e) + e->length;
> + p = acpi_os_map_iomem(pa_v5, v66param_size);
> }
> +
> return p;
> }
> }
> @@ -527,6 +539,7 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> u64 param3, u64 param4)
> {
> struct apei_exec_context ctx;
> + u32 param_size = is_v2 ? v66param_size : v5param_size;
> u64 val, trigger_paddr, timeout = FIRMWARE_TIMEOUT;
> int i, rc;
>
> @@ -539,11 +552,11 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> if (acpi5) {
> struct set_error_type_with_address *v5param;
>
> - v5param = kmalloc(v5param_size, GFP_KERNEL);
> + v5param = kmalloc(param_size, GFP_KERNEL);
> if (!v5param)
> return -ENOMEM;
>
> - memcpy_fromio(v5param, einj_param, v5param_size);
> + memcpy_fromio(v5param, einj_param, param_size);
> v5param->type = type;
> if (type & ACPI5_VENDOR_BIT) {
> switch (vendor_flags) {
> @@ -601,7 +614,7 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> break;
> }
> }
> - memcpy_toio(einj_param, v5param, v5param_size);
> + memcpy_toio(einj_param, v5param, param_size);
> kfree(v5param);
> } else {
> rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
> @@ -1132,9 +1145,14 @@ static void einj_remove(struct faux_device *fdev)
> struct apei_exec_context ctx;
>
> if (einj_param) {
> - acpi_size size = (acpi5) ?
> - v5param_size :
> - sizeof(struct einj_parameter);
> + acpi_size size;
> +
> + if (v66param_size)
> + size = v66param_size;
> + else if (acpi5)
> + size = v5param_size;
> + else
> + size = sizeof(struct einj_parameter);
>
> acpi_os_unmap_iomem(einj_param, size);
> if (vendor_errors.size)
> --
Applied as 6.18-rc material, thanks!
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-11-19 12:35 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-19 1:27 [PATCH] ACPI: APEI: EINJ: Fix EINJV2 initialization and injection Tony Luck
2025-11-19 12:34 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®