linux-kernel - Re: [PATCH v2] ACPI: APEI: EINJ: warn on invalid argument when explicitly indicated by platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 27 Mar 2023 20:47:20 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Shuai Xue <xueshuai@...ux.alibaba.com>
Cc:     tony.luck@...el.com, baolin.wang@...ux.alibaba.com,
        benjamin.cheatham@....com, bp@...en8.de, dan.j.williams@...el.com,
        james.morse@....com, lenb@...nel.org, linux-acpi@...r.kernel.org,
        linux-kernel@...r.kernel.org, rafael@...nel.org,
        zhuo.song@...ux.alibaba.com
Subject: Re: [PATCH v2] ACPI: APEI: EINJ: warn on invalid argument when
 explicitly indicated by platform

On Thu, Mar 23, 2023 at 2:54 AM Shuai Xue <xueshuai@...ux.alibaba.com> wrote:
>
> OSPM executes an EXECUTE_OPERATION action to instruct the platform to begin
> the injection operation, then executes a GET_COMMAND_STATUS action to
> determine the status of the completed operation. The ACPI Specification
> documented error codes[1] are:
>
>         0 = Success (Linux #define EINJ_STATUS_SUCCESS)
>         1 = Unknown failure (Linux #define EINJ_STATUS_FAIL)
>         2 = Invalid Access (Linux #define EINJ_STATUS_INVAL)
>
> The original code report -EBUSY for both "Unknown Failure" and "Invalid
> Access" cases. Actually, firmware could do some platform dependent sanity
> checks and returns different error codes, e.g. "Invalid Access" to indicate
> to the user that the parameters they supplied cannot be used for injection.
>
> To this end, fix to return -EINVAL in the __einj_error_inject() error
> handling case instead of always -EBUSY, when explicitly indicated by the
> platform in the status of the completed operation.
>
> [1] ACPI Specification 6.5 18.6.1. Error Injection Table
>
> Signed-off-by: Shuai Xue <xueshuai@...ux.alibaba.com>
> Reviewed-by: Tony Luck <tony.luck@...el.com>
> ---
> changelog since v1:
> - elaborate commit log based on follow up discussion with Tony
> - pick up Reviewed-by tag of Tony
> ---
>  drivers/acpi/apei/einj.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
> index b4373e575660..fa0b4320312e 100644
> --- a/drivers/acpi/apei/einj.c
> +++ b/drivers/acpi/apei/einj.c
> @@ -489,9 +489,15 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>         if (rc)
>                 return rc;
>         val = apei_exec_ctx_get_output(&ctx);
> -       if (val != EINJ_STATUS_SUCCESS)
> +       if (val == EINJ_STATUS_FAIL)
>                 return -EBUSY;
> +       else if (val == EINJ_STATUS_INVAL)
> +               return -EINVAL;
>
> +       /*
> +        * The error is injected into the platform successfully, then it needs
> +        * to trigger the error.
> +        */
>         rc = apei_exec_run(&ctx, ACPI_EINJ_GET_TRIGGER_TABLE);
>         if (rc)
>                 return rc;
> --

Applied as 6.4 material, thanks!