| 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
| ||
|
Message-ID: <1AE640813FDE7649BE1B193DEA596E883BBB6FD4@SHSMSX101.ccr.corp.intel.com>
Date: Wed, 1 Jun 2016 01:30:47 +0000
From: "Zheng, Lv" <lv.zheng@...el.com>
To: Mike Marshall <hubcap@...ibond.com>
CC: Boris Ostrovsky <boris.ostrovsky@...cle.com>,
Lv Zheng <zetalog@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
"Brown, Len" <len.brown@...el.com>,
"Moore, Robert" <robert.moore@...el.com>
Subject: RE: [RFC PATCH v2] ACPICA / Hardware: Fix old register check in
acpi_hw_get_access_bit_width()
Hi,
> From: Mike Marshall [mailto:hubcap@...ibond.com]
> Subject: Re: [RFC PATCH v2] ACPICA / Hardware: Fix old register check in
> acpi_hw_get_access_bit_width()
>
> Hi Lv...
>
> I was dead in the water before this patch, qemu-kvm would crash
> right away, now everything seems to work great again, thanks! From
> my perspective this fixes the c3bc26d problem.
[Lv Zheng]
Great.
The bisection result is c3bc26d, but the code is actually upstreamed in b314a172.
c3bc26d is the first commit enabled the bug. :)
>
> Acked-by: Mike Marshall <hubcap@...ibond.com>
[Lv Zheng]
Thanks for the test.
Best regards
-Lv
>
> -Mike
>
> On Tue, May 31, 2016 at 3:13 AM, Zheng, Lv <lv.zheng@...el.com> wrote:
> > Hi, Boris and Mike
> >
> > Please help to validate if this version can also fix your issues.
> > After enumerating the possible cases, I realized that the address check
> might not be necessary.
> > But we need a max_bit_width check in this function to make it prepared
> for a future usage in acpi_read()/acpi_write().
> > Thanks in advance.
> >
> > Best regards
> > -Lv
> >
> >> From: Zheng, Lv
> >> Subject: [RFC PATCH v2] ACPICA / Hardware: Fix old register check in
> >> acpi_hw_get_access_bit_width()
> >>
> >> The address check in acpi_hw_get_access_bit_width() should be byte
> >> width
> >> based, not bit width based. This patch fixes this mistake.
> >>
> >> For those who want to review acpi_hw_access_bit_width(), here is the
> >> concerns and the design details of the function:
> >>
> >> It is supposed that the GAS Address field should be aligned to the byte
> >> width indicated by the GAS AccessSize field. Similarly, for the old non
> >> GAS register, it is supposed that its Address should be aligned to its
> >> Length.
> >> For the "AccessSize = 0 (meaning ANY)" case, we try to return the
> >> maximum
> >> instruction width (64 for MMIO or 32 for PIO) or the user expected
> access
> >> bit width (64 for acpi_read()/acpi_write() or 32 for acpi_hw_read()/
> >> acpi_hw_write()) for futher operation and it is supposed that the GAS
> >> Address field should always be aligned to the maximum expected
> access
> >> bit
> >> width (otherwise it can't be ANY).
> >>
> >> The problem is in acpi_tb_init_generic_address(), where the non GAS
> >> register's Length is converted into the GAS BitWidth field, its Address is
> >> converted into the GAS Address field, and the GAS AccessSize field is
> left
> >> 0 but most of the register actually cannot be accessed using "ANY"
> >> accesses.
> >>
> >> As a conclusion, when AccessSize = 0 (ANY), the Address should either
> be
> >> aligned to the BitWidth (wrong conversion) or aligned to 32 (PIO) or 64
> >> (MMIO). Since BitWidth for the wrong conversion is 8,16,32, the
> Address
> >> of the real GAS should always be aligned to 8,16,32, the address
> alignment
> >> check is not necessary. But we in fact could enhance the check for a
> future
> >> case where max_bit_width could be 64 for a PIO access issued from
> >> acpi_read()/acpi_write().
> >>
> >> Fixes: b314a172ee96 ("ACPICA: Hardware: Add optimized access bit
> width
> >> support")
> >> Cc: Boris Ostrovsky <boris.ostrovsky@...cle.com>
> >> Cc: Mike Marshall <hubcap@...ibond.com>
> >> Suggested-by: Jan Beulich <jbeulich@...e.com>
> >> Signed-off-by: Lv Zheng <lv.zheng@...el.com>
> >> ---
> >> drivers/acpi/acpica/hwregs.c | 16 +++++++---------
> >> 1 file changed, 7 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/acpi/acpica/hwregs.c b/drivers/acpi/acpica/hwregs.c
> >> index 0f18dbc..0553c0b 100644
> >> --- a/drivers/acpi/acpica/hwregs.c
> >> +++ b/drivers/acpi/acpica/hwregs.c
> >> @@ -86,24 +86,22 @@ acpi_hw_get_access_bit_width(struct
> >> acpi_generic_address *reg, u8 max_bit_width)
> >> u64 address;
> >>
> >> if (!reg->access_width) {
> >> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> >> + max_bit_width = 32;
> >> + }
> >> /*
> >> * Detect old register descriptors where only the bit_width
> >> field
> >> * makes senses. The target address is copied to handle
> >> possible
> >> * alignment issues.
> >> */
> >> ACPI_MOVE_64_TO_64(&address, ®->address);
> >> - if (!reg->bit_offset && reg->bit_width &&
> >> + if (reg->bit_width < max_bit_width &&
> >> + !reg->bit_offset && reg->bit_width &&
> >> ACPI_IS_POWER_OF_TWO(reg->bit_width) &&
> >> - ACPI_IS_ALIGNED(reg->bit_width, 8) &&
> >> - ACPI_IS_ALIGNED(address, reg->bit_width)) {
> >> + ACPI_IS_ALIGNED(reg->bit_width, 8)) {
> >> return (reg->bit_width);
> >> - } else {
> >> - if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO)
> >> {
> >> - return (32);
> >> - } else {
> >> - return (max_bit_width);
> >> - }
> >> }
> >> + return (max_bit_width);
> >> } else {
> >> return (1 << (reg->access_width + 2));
> >> }
> >> --
> >> 1.7.10
> >
Powered by blists - more mailing lists