linux-kernel - Re: [PATCH 8/9] PCI: Ignore BAR contents when firmware left decoding disabled

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACVXFVMz+6DaSgT0EHeVfyBOsXfWDYnZQ0wy7pwcey5ahM6rUA@mail.gmail.com>
Date:	Fri, 14 Mar 2014 09:48:35 +0800
From:	Ming Lei <tom.leiming@...il.com>
To:	Bjorn Helgaas <bhelgaas@...gle.com>
Cc:	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Rusty Russell <rusty@...tcorp.com.au>
Subject: Re: [PATCH 8/9] PCI: Ignore BAR contents when firmware left decoding disabled

On Fri, Mar 14, 2014 at 12:08 AM, Bjorn Helgaas <bhelgaas@...gle.com> wrote:
> On Thu, Mar 13, 2014 at 2:51 AM, Ming Lei <tom.leiming@...il.com> wrote:
>> Hi Bjorn,
>>
>> I found this patch broke virtio-pci devices.
>
> Thanks a lot for testing this.
>
>> On Thu, Feb 27, 2014 at 3:37 AM, Bjorn Helgaas <bhelgaas@...gle.com> wrote:
>>> Don't rely on BAR contents when the command register says the BAR is
>>> disabled.
>>>
>>> If we receive a PCI device from firmware (or a hot-added device that was
>>> just powered up) with the MEMORY or IO enable bits in the PCI command
>>> register cleared, there's no reason to believe the BARs contain valid
>>> addresses.
>>
>> From PCI LOCAL BUS SPECIFICATION, REV. 3.0, both
>> PCI_COMMAND_IO and PCI_COMMAND_MEM should be
>> cleared after reset, so looks the patch sets IORESOURCE_UNSET
>> too early because PCI drivers may call pci_enable_device()
>> (->pci_enable_resources()) to enable the two bits of
>> PCI_COMMAND explicitly.
>
> The point is that it's not safe to enable those two bits unless we're
> certain that the BARs they control contain valid values that don't
> conflict with anything else in the system.
>
> Maybe we should only set IORESOURCE_UNSET when we find a conflict or a
> BAR that's not contained by an upstream bridge window, and we should
> try to reallocate then.  I'm pretty sure we do that at least in some
> cases, but it would probably simplify things if we did it more
> consistently, and maybe we shouldn't set it at all here in
> __pci_read_base().

I think so because __pci_read_base() is called in device emulation
path.

>
> But I'd like to understand your situation better, so can you provide
> more details, please?  Complete before/after dmesg logs would go a
> long way toward illustrating the problem you're seeing.

Please see the two attachment log. The memory allocation failure
is caused by mistaken value read from pci address after the device
is failed to enable.


Thanks,
-- 
Ming Lei

Download attachment "dmesg_before_revert.tar.gz" of type "application/x-gzip" (6458 bytes)

Download attachment "dmesg_after_revert.tar.gz" of type "application/x-gzip" (10732 bytes)