linux-kernel - Re: [PATCH v2 08/13] ACPICA: Hardware: Add optimized access bit width support

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:	Fri, 27 May 2016 13:31:36 -0400
From:	Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:	"Zheng, Lv" <lv.zheng@...el.com>, Jan Beulich <jbeulich@...e.com>
Cc:	"ian.jackson@...citrix.com" <ian.jackson@...citrix.com>,
	"zetalog@...il.com" <zetalog@...il.com>,
	"Brown, Len" <len.brown@...el.com>,
	"Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
	"Moore, Robert" <robert.moore@...el.com>,
	"xen-devel@...ts.xen.org" <xen-devel@...ts.xen.org>,
	"rjw@...ysocki.net" <rjw@...ysocki.net>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 08/13] ACPICA: Hardware: Add optimized access bit width
 support

On 05/27/2016 03:34 AM, Zheng, Lv wrote:
> Hi,
>
>> From: Boris Ostrovsky [mailto:boris.ostrovsky@...cle.com]
>> Subject: Re: [PATCH v2 08/13] ACPICA: Hardware: Add optimized access
>> bit width support
>>
>> On 05/26/2016 12:26 PM, Jan Beulich wrote:
>>>>>> Boris Ostrovsky <boris.ostrovsky@...cle.com> 05/25/16 9:17
>> PM >>>
>>>> On 05/05/2016 12:58 AM, Lv Zheng wrote:
>>>>> +static u8
>>>>> +acpi_hw_get_access_bit_width(struct acpi_generic_address *reg, u8
>> max_bit_width)
>>>>> +{
>>>>> +    u64 address;
>>>>> +
>>>>> +    if (!reg->access_width) {
>>>>> +        /*
>>>>> +         * 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, &reg->address);
>>>>> +        if (!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)) {
>>>>> +            return (reg->bit_width);
>>>>> +        } else {
>>>>> +            if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
>>>>> +                return (32);
>>>> This (together with "... Add access_width/bit_offset support in
>>>> acpi_hw_write") breaks Xen guests using older QEMU which doesn't
>> support
>>>> 4-byte IO accesses.
>>>>
>>>> Why not return "reg->bit_width?:max_bit_width" ? This will preserve
>>>> original behavior.
>>> Did you figure out why we get here in the first place, instead of taking
>> the
>>> first "return"? I.e. isn't the issue the apparently wrong use of the second
>>> ACPI_IS_ALIGNED() above? Afaict it ought to be
>>> ACPI_IS_ALIGNED(address, reg->bit_width / 8)...
>> We are trying to access address 0x...b004 (PM1a control) so yes, fixing
>> alignment check would probably resolve the problem that we are seeing
>> now.
>>
>> However, for compatibility purposes we may consider not doing any
>> checks
>> and simply return bit_width if access_width is not available.
> [Lv Zheng] 

Your patch from earlier does resolve both issues, thanks.

> Your solution could result in problems like:
> If a GAS is defined with bit_width not a power of 2, and access_width is any (0).
>
> So the correct fix here is to make sure if bit_width is exactly 8,16,32,64, which matches old register descriptors.
> I added address check here because I want to limit this regression safe check to old register descriptors.
> As since the old bit_width can actually reflect the register's access width, the address of the register should always be aligned.
>
> There might be cases that using the new GAS register descriptor format, it is possible to define a GAS that is not aligned, and it's bit_width is exactly 8,16,32,64.
> We shouldn't make a default access_width decision using bit_width here.

True. But OTOH switching to 32-bit accesses may result in us suddenly
trying to touch bytes we haven't accessed before.

-boris


> The address check here can help to avoid applying this workaround on such cases.
>
> Thanks and best regards
> -Lv