Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161151AbbKEMyJ (ORCPT ); Thu, 5 Nov 2015 07:54:09 -0500 Received: from mail-lb0-f173.google.com ([209.85.217.173]:34582 "EHLO mail-lb0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1032444AbbKEMyG (ORCPT ); Thu, 5 Nov 2015 07:54:06 -0500 Subject: Re: [Bugfix v4] PCI, ACPI: Fix regressions caused by resource_size_t overflow with 32-bit kernel To: Jiang Liu References: <559107F2.3080701@pr.hu> <1436340399-19695-1-git-send-email-jiang.liu@linux.intel.com> <563780FC.5070503@linaro.org> Cc: Tomasz Nowicki , "Rafael J . Wysocki" , Bjorn Helgaas , Ingo Molnar , Boszormenyi Zoltan , Len Brown , LKML , linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org, "x86 @ kernel . org" From: Tomasz Nowicki Message-ID: <563B5161.1060105@semihalf.com> Date: Thu, 5 Nov 2015 13:53:53 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <563780FC.5070503@linaro.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5168 Lines: 118 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 >> Fixes: 593669c2ac0f ("x86/PCI/ACPI: Use common ACPI resource >> interfaces to simplify implementation") >> Signed-off-by: Jiang Liu >> Cc: stable@vger.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/