linux-kernel - Re: [PATCH] acpi: zero-initialize acpi

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0hbg_5VLCT3cXgK4WkCTwNAUGrUuRe66DoCHf6xydsTzQ@mail.gmail.com>
Date: Mon, 21 Oct 2024 13:31:39 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Payam Moradshahi <payamm@...gle.com>
Cc: "Rafael J . Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>, linux-acpi@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] acpi: zero-initialize acpi_object union structure

On Thu, Oct 17, 2024 at 5:59 AM Payam Moradshahi <payamm@...gle.com> wrote:
>
> The way in which acpi_object union is being initialized varies based on
> compiler type, version and flags used. Some will zero-initialize the
> entire union structure and some will only initialize the first N-bytes
> of the union structure.

Any details?

> This could lead to uninitialized union members.

So this is working around a compiler bug AFAICS.

If the compiler has this bug, is it guaranteed to compile the rest of
the kernel correctly?

> This bug was confirmed by observing non-zero value for object->processor
> structure variables.

Where has it been observed?  What compiler version(s)? etc.

> non-zero initialized members of acpi_object union structure causes
> incorrect error reporting by the driver.
>
> If a BIOS is using "Device" statement as opposed to "Processor"
> statement, object variable may contain uninitialized members causing the
> driver to report "Invalid PBLK length" incorrectly.
>
> Using memset to zero-initialize the union structure fixes this issue and
> also removes the dependency of this function on compiler versions and
> flags being used.
>
> Tested: Tested on ARM64 hardware that was printing this error and
> confirmed the prints were gone.
>
> Also confirmed this does not cause regression on ARM64 and X86
> machines.
>
> Signed-off-by: Payam Moradshahi <payamm@...gle.com>
> ---
>  drivers/acpi/acpi_processor.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> index 7cf6101cb4c73..6696ad4937d21 100644
> --- a/drivers/acpi/acpi_processor.c
> +++ b/drivers/acpi/acpi_processor.c
> @@ -275,7 +275,7 @@ static inline int acpi_processor_hotadd_init(struct acpi_processor *pr,
>
>  static int acpi_processor_get_info(struct acpi_device *device)
>  {
> -       union acpi_object object = { 0 };
> +       union acpi_object object;
>         struct acpi_buffer buffer = { sizeof(union acpi_object), &object };
>         struct acpi_processor *pr = acpi_driver_data(device);
>         int device_declaration = 0;
> @@ -284,6 +284,8 @@ static int acpi_processor_get_info(struct acpi_device *device)
>         unsigned long long value;
>         int ret;
>
> +       memset(&object, 0, sizeof(union acpi_object));
> +
>         acpi_processor_errata();
>
>         /*
> --