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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CACVXFVPq2nQ8jcn+v424roieO+v_asyfXcHUHv=uSOABYHJ4eg@mail.gmail.com>
Date:	Thu, 20 Mar 2014 09:32:52 +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>,
	Pekka Enberg <penberg@...nel.org>,
	Sasha Levin <sasha.levin@...cle.com>
Subject: Re: [PATCH 8/9] PCI: Ignore BAR contents when firmware left decoding disabled

On Thu, Mar 20, 2014 at 12:45 AM, Bjorn Helgaas <bhelgaas@...gle.com> wrote:
> On Tue, Mar 18, 2014 at 10:52 PM, Ming Lei <tom.leiming@...il.com> wrote:
>> Hi,
>>
>> Looks Sasha fixed the problem in lkvm tool[1].
>>
>> Sasha, looks we both saw the problem, but from technical
>> view, I am wondering if the fix is correct, because PCI spec.
>> requires that the IO/MMIO bits in COMMAND register should
>> be cleared after reset, maybe there are some potential problem
>> in lkvm pci emulation.
>
> I think I'm going to revert this patch ([2], "Ignore BAR contents when
> firmware left decoding disabled").  The main reason for that patch was
> to try for a consistent way of figuring out whether BARs are valid
> that we could use on all architectures, but I think we can do it in a
> better way.
>
> That said, this kvm change should not be necessary.  We *should* be
> able to take any PCI device and initialize it from power-on state
> without any dependencies on what the BIOS left in the BARs or the
> command register.  As far as I can tell, the PCI core actually worked
> fine in this case (we assigned valid addresses to the devices), but
> something else blew up.  If I revert that patch, it will cover up
> whatever this other bug is, but it would be much better to figure out
> what it is and fix is.
>
> You said earlier that "The memory allocation failure is caused by
> mistaken value read from pci address after the device is failed to
> enable."  Can you elaborate on that?  Are you saying that something

Sorry, that's my take for granted.

> tried to read from a region mapped by a BAR even though
> pci_enable_device() failed?  That would be a programming error, of
> course.  If you have any more details about exactly where this
> happened, that would help a lot in finding the problem.

When I check again, as you saw in the dmesg log after reverting, the
virtio device has been enabled successfully, looks no obvious PCI
failure, and the only problem is that the virtio driver reads zero queue
number from one region mapped by a BAR:

ioread16(vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NUM)
         <- setup_vq(): drivers/virtio/virtio_pci.c

That causes the memory allocation failure.

Thanks,
-- 
Ming Lei
--
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/