| 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: <563B5161.1060105@semihalf.com>
Date: Thu, 5 Nov 2015 13:53:53 +0100
From: Tomasz Nowicki <tn@...ihalf.com>
To: Jiang Liu <jiang.liu@...ux.intel.com>
Cc: Tomasz Nowicki <tomasz.nowicki@...aro.org>,
"Rafael J . Wysocki" <rjw@...ysocki.net>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Ingo Molnar <mingo@...nel.org>,
Boszormenyi Zoltan <zboszor@...hu>,
Len Brown <lenb@...nel.org>,
LKML <linux-kernel@...r.kernel.org>, linux-pci@...r.kernel.org,
linux-acpi@...r.kernel.org, "x86 @ kernel . org" <x86@...nel.org>
Subject: Re: [Bugfix v4] PCI, ACPI: Fix regressions caused by resource_size_t
overflow with 32-bit kernel
On 02.11.2015 16:27, Tomasz Nowicki wrote:
> On 08.07.2015 09:26, Jiang Liu wrote:
>> Zoltan Boszormenyi reported this regression:
>> "There's a Realtek RTL8111/8168/8411 (PCI ID 10ec:8168, Subsystem ID
>> 1565:230e) network chip on the mainboard. After the r8169 driver
>> loaded
>> the IRQs in the machine went berserk. Keyboard keypressed arrived
>> with
>> considerable latency and duplicated, so no real work was possible.
>> The machine responded to the power button but didn't actually power
>> down. It just stuck at the powering down message. I had to press the
>> power button for 4 seconds to power it down.
>>
>> The computer is a POS machine with a big battery inside. Because
>> of this,
>> either ACPI or the Realtek chip kept the bad state and after
>> rebooting,
>> the network chip didn't even show up in lspci. Not even the PXE ROM
>> announced itself during boot. I had to disconnect the battery to beat
>> some sense back to the computer.
>>
>> The regression happens with 4.0.5, 4.1.0-rc8 and 4.1.0-final.
>> 3.18.16 was
>> good."
>>
>> The regression is caused by commit 593669c2ac0f ("x86/PCI/ACPI: Use
>> common
>> ACPI resource interfaces to simplify implementation"). Since commit
>> 593669c2ac0f, x86 PCI ACPI host bridge driver validates ACPI resources by
>> first converting an ACPI resource to a 'struct resource' structure and
>> then applying checks against the converted resource structure. The
>> 'start'
>> and 'end' fields in 'struct resource' are defined to be type of
>> resource_size_t, which may be 32 bits or 64 bits depending on
>> CONFIG_PHYS_ADDR_T_64BIT.
>>
>> This may cause incorrect resource validation results with 32-bit kernels
>> because 64-bit ACPI resource descriptors may get truncated when
>> converting
>> to 32-bit 'start' and 'end' fields in 'struct resource'. It eventually
>> affects PCI resource allocation subsystem and makes some PCI devices and
>> the system behave abnormally due to incorrect resource assignment.
>>
>> So enhance the ACPI resource parsing interfaces to ignore ACPI resource
>> descriptors with address/offset above 4G when running in 32-bit mode.
>>
>> With the fix applied, the behavior of the machine was restored to how
>> 3.18.16 worked, i.e. the memory range that is over 4GB is ignored again,
>> and lspci -vvxxx shows that everything is at the same memory window as
>> they were with 3.18.16.
>>
>> Reported-and-Tested-by: Boszormenyi Zoltan <zboszor@...hu>
>> Fixes: 593669c2ac0f ("x86/PCI/ACPI: Use common ACPI resource
>> interfaces to simplify implementation")
>> Signed-off-by: Jiang Liu <jiang.liu@...ux.intel.com>
>> Cc: stable@...r.kernel.org # 4.0
>> ---
>> drivers/acpi/resource.c | 24 +++++++++++++++---------
>> 1 file changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index 10561ce16ed1..e8d281739cbc 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -194,6 +194,7 @@ static bool acpi_decode_space(struct resource_win
>> *win,
>> u8 iodec = attr->granularity == 0xfff ? ACPI_DECODE_10 :
>> ACPI_DECODE_16;
>> bool wp = addr->info.mem.write_protect;
>> u64 len = attr->address_length;
>> + u64 start, end, offset = 0;
>> struct resource *res = &win->res;
>>
>> /*
>> @@ -205,9 +206,6 @@ static bool acpi_decode_space(struct resource_win
>> *win,
>> pr_debug("ACPI: Invalid address space min_addr_fix %d,
>> max_addr_fix %d, len %llx\n",
>> addr->min_address_fixed, addr->max_address_fixed, len);
>>
>> - res->start = attr->minimum;
>> - res->end = attr->maximum;
>> -
>> /*
>> * For bridges that translate addresses across the bridge,
>> * translation_offset is the offset that must be added to the
>> @@ -215,12 +213,22 @@ static bool acpi_decode_space(struct
>> resource_win *win,
>> * primary side. Non-bridge devices must list 0 for all Address
>> * Translation offset bits.
>> */
>> - if (addr->producer_consumer == ACPI_PRODUCER) {
>> - res->start += attr->translation_offset;
>> - res->end += attr->translation_offset;
>> - } else if (attr->translation_offset) {
>> + if (addr->producer_consumer == ACPI_PRODUCER)
>> + offset = attr->translation_offset;
>> + else if (attr->translation_offset)
>> pr_debug("ACPI: translation_offset(%lld) is invalid for
>> non-bridge device.\n",
>> attr->translation_offset);
>> + start = attr->minimum + offset;
>> + end = attr->maximum + offset;
>
> I still see the issue for this area, I mean ACPI_IO_RANGE. You are
> adding translation offset to attr->minimum, build resource structure
> which is then passed to acpi_dev_ioresource_flags and compared against
> 0x10003. It causes some IO ranges to be ignored.
>
Kindly reminder, any comments?
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists