| Message ID | 20210916194957.197119-1-mlangsdo@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | ACPI: SPCR: check if table->serial_port.access_width is too wide | expand |
On Thu, Sep 16, 2021 at 9:50 PM Mark Langsdorf <mlangsdo@redhat.com> wrote: > > If table->serial_port.access_width is more than 29, it causes > undefined behavior when ACPI_ACCESS_BIT_WIDTH shifts it to > (1 << ((size) + 2)): > > [ 0.000000] UBSAN: Undefined behaviour in drivers/acpi/spcr.c:114:11 > [ 0.000000] shift exponent 102 is too large for 32-bit type 'int' > > Test that serial_port.access_width is less than 30 and set it to 6 > if it is not. > > Signed-off-by: Mark Langsdorf <mlangsdo@redhat.com> This consists of a Linux part and an ACPICA part (the changes in actypes.h). The ACPICA part needs to be submitted to the upstream project before applying this. > --- > drivers/acpi/spcr.c | 8 ++++++-- > include/acpi/actypes.h | 10 ++++++++-- > 2 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c > index 134f7f60cf8102..52f17d631d24e9 100644 > --- a/drivers/acpi/spcr.c > +++ b/drivers/acpi/spcr.c > @@ -111,8 +111,12 @@ int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console) > pr_info("SPCR table version %d\n", table->header.revision); > > if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) { > - switch (ACPI_ACCESS_BIT_WIDTH(( > - table->serial_port.access_width))) { > + u32 bit_width = table->serial_port.access_width; > + if(bit_width > ACPI_ACCESS_BIT_MAX) { > + pr_err("Unacceptable wide SPCR Access Width. Defaulting to byte size\n"); > + bit_width = ACPI_ACCESS_BIT_DEFAULT; > + } > + switch (ACPI_ACCESS_BIT_WIDTH((bit_width))) { > default: > pr_err("Unexpected SPCR Access Width. Defaulting to byte size\n"); > fallthrough; > diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h > index 92c71dfce0d5d9..cefbb7ad253e02 100644 > --- a/include/acpi/actypes.h > +++ b/include/acpi/actypes.h > @@ -536,8 +536,14 @@ typedef u64 acpi_integer; > * Can be used with access_width of struct acpi_generic_address and access_size of > * struct acpi_resource_generic_register. > */ > -#define ACPI_ACCESS_BIT_WIDTH(size) (1 << ((size) + 2)) > -#define ACPI_ACCESS_BYTE_WIDTH(size) (1 << ((size) - 1)) > +#define ACPI_ACCESS_BIT_SHIFT 2 > +#define ACPI_ACCESS_BYTE_SHIFT -1 > +#define ACPI_ACCESS_BIT_MAX (31 - ACPI_ACCESS_BIT_SHIFT) > +#define ACPI_ACCESS_BYTE_MAX (31 - ACPI_ACCESS_BYTE_SHIFT) > +#define ACPI_ACCESS_BIT_DEFAULT (8 - ACPI_ACCESS_BIT_SHIFT) > +#define ACPI_ACCESS_BYTE_DEFAULT (8 - ACPI_ACCESS_BYTE_SHIFT) > +#define ACPI_ACCESS_BIT_WIDTH(size) (1 << ((size) + ACPI_ACCESS_BIT_SHIFT)) > +#define ACPI_ACCESS_BYTE_WIDTH(size) (1 << ((size) + ACPI_ACCESS_BYTE_SHIFT)) > > /******************************************************************************* > * > -- > 2.26.3 >
On 9/24/21 11:10 AM, Rafael J. Wysocki wrote: > On Thu, Sep 16, 2021 at 9:50 PM Mark Langsdorf <mlangsdo@redhat.com> wrote: >> If table->serial_port.access_width is more than 29, it causes >> undefined behavior when ACPI_ACCESS_BIT_WIDTH shifts it to >> (1 << ((size) + 2)): >> >> [ 0.000000] UBSAN: Undefined behaviour in drivers/acpi/spcr.c:114:11 >> [ 0.000000] shift exponent 102 is too large for 32-bit type 'int' >> >> Test that serial_port.access_width is less than 30 and set it to 6 >> if it is not. >> >> This consists of a Linux part and an ACPICA part (the changes in >> actypes.h). The ACPICA part needs to be submitted to the upstream >> project before applying this. Thanks, I wasn't sure of the procedure. I'll get the ACPICA part accepted and come back with the rest. --Mark Langsdorf
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c index 134f7f60cf8102..52f17d631d24e9 100644 --- a/drivers/acpi/spcr.c +++ b/drivers/acpi/spcr.c @@ -111,8 +111,12 @@ int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console) pr_info("SPCR table version %d\n", table->header.revision); if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) { - switch (ACPI_ACCESS_BIT_WIDTH(( - table->serial_port.access_width))) { + u32 bit_width = table->serial_port.access_width; + if(bit_width > ACPI_ACCESS_BIT_MAX) { + pr_err("Unacceptable wide SPCR Access Width. Defaulting to byte size\n"); + bit_width = ACPI_ACCESS_BIT_DEFAULT; + } + switch (ACPI_ACCESS_BIT_WIDTH((bit_width))) { default: pr_err("Unexpected SPCR Access Width. Defaulting to byte size\n"); fallthrough; diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h index 92c71dfce0d5d9..cefbb7ad253e02 100644 --- a/include/acpi/actypes.h +++ b/include/acpi/actypes.h @@ -536,8 +536,14 @@ typedef u64 acpi_integer; * Can be used with access_width of struct acpi_generic_address and access_size of * struct acpi_resource_generic_register. */ -#define ACPI_ACCESS_BIT_WIDTH(size) (1 << ((size) + 2)) -#define ACPI_ACCESS_BYTE_WIDTH(size) (1 << ((size) - 1)) +#define ACPI_ACCESS_BIT_SHIFT 2 +#define ACPI_ACCESS_BYTE_SHIFT -1 +#define ACPI_ACCESS_BIT_MAX (31 - ACPI_ACCESS_BIT_SHIFT) +#define ACPI_ACCESS_BYTE_MAX (31 - ACPI_ACCESS_BYTE_SHIFT) +#define ACPI_ACCESS_BIT_DEFAULT (8 - ACPI_ACCESS_BIT_SHIFT) +#define ACPI_ACCESS_BYTE_DEFAULT (8 - ACPI_ACCESS_BYTE_SHIFT) +#define ACPI_ACCESS_BIT_WIDTH(size) (1 << ((size) + ACPI_ACCESS_BIT_SHIFT)) +#define ACPI_ACCESS_BYTE_WIDTH(size) (1 << ((size) + ACPI_ACCESS_BYTE_SHIFT)) /******************************************************************************* *
If table->serial_port.access_width is more than 29, it causes undefined behavior when ACPI_ACCESS_BIT_WIDTH shifts it to (1 << ((size) + 2)): [ 0.000000] UBSAN: Undefined behaviour in drivers/acpi/spcr.c:114:11 [ 0.000000] shift exponent 102 is too large for 32-bit type 'int' Test that serial_port.access_width is less than 30 and set it to 6 if it is not. Signed-off-by: Mark Langsdorf <mlangsdo@redhat.com> --- drivers/acpi/spcr.c | 8 ++++++-- include/acpi/actypes.h | 10 ++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-)