| Message ID | 20241017035940.4067922-1-payamm@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | acpi: zero-initialize acpi_object union structure | expand |
On Thu, Oct 17, 2024 at 5:59 AM Payam Moradshahi <payamm@google.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@google.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(); > > /* > --
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(); /*
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. This could lead to uninitialized union members. This bug was confirmed by observing non-zero value for object->processor structure variables. 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@google.com> --- drivers/acpi/acpi_processor.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)