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]
Date:   Tue, 24 Jan 2017 16:42:37 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     Marc Zyngier <marc.zyngier@....com>,
        Shameerali Kolothum Thodi 
        <shameerali.kolothum.thodi@...wei.com>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "will.deacon@....com" <will.deacon@....com>,
        "eric.auger@...hat.com" <eric.auger@...hat.com>
Cc:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Linuxarm <linuxarm@...wei.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        John Garry <john.garry@...wei.com>,
        "Guohanjun (Hanjun Guo)" <guohanjun@...wei.com>
Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum
 161010801

On 24/01/17 16:29, Marc Zyngier wrote:
> On 24/01/17 16:14, Shameerali Kolothum Thodi wrote:
>>>>>> Let's contemplate this for a moment. If we're on the affected ITS,
>>>>> we're
>>>>>> using the physical address of the GITS_TRANSLATER register. What
>>>>>> guarantees that this is not going to conflict with an IOVA that DMA
>>>>> is
>>>>>> going to use? From looking at these patches, my feeling is "not
>>>>> much".
>>>>>>
>>>>>> So if I'm right, you're opening the door to some interesting memory
>>>>>> corruption if the two regions ever intersect.
>>>>>>
>>>>>> Robin, what do you think?
>>>>>
>>>>> Yup. Unless the ITS physical address is actually reserved from the
>>>>> IOVA domain, it's still free to be allocated for DMA mappings, and
>>> if
>>>>> that ever happens then you'll get odd bits of data landing in the
>>> ITS
>>>>> instead of RAM, and maybe even locked-up devices or worse if the
>>>>> doorbell gives back decode errors on read attempts. It's essentially
>>>>> the exact same problem as we have with memory-mapped PCI windows,
>>> and
>>>>> needs to be solved in the same fashion, i.e. between the SMMU and
>>> the
>>>>> IOMMU-DMA code.
>>>>
>>>> Is this something that can incorporated in Eric's latest patch
>>> series[1]?
>>>> It does mentions reserved regions can be:
>>>> - directly mapped regions
>>>> - regions that cannot be iommu mapped (PCI host bridge windows, ...)
>>>> - MSI regions (because they belong to another address space or
>>> because
>>>>   they are not translated by the IOMMU and need special handling)
>>>>
>>>> Though I am not clear our case comes under "the MSI regions that are
>>>> not translated by the IOMMU and need special handling" or not.
>>>
>>> Well, given that in your case, the IOMMU never sees the MSI write, it
>>> definitely falls into the "not translated" category.
>>>
>>> As for handling it on top of Eric's series, that's probably the most
>>> reasonable thing to do, which also means that none of this should
>>> appear in the ITS driver. Robin seems to have an idea on how to
>>> approach this.
>>
>> Ok. Thanks for that Marc/Robin.
>>
>> But I am not sure we can get away with ITS driver. Because current vfio
>> patch series[1] treats GICV3 ITS as irq safe and is setting 
>> IRQ_DOMAIN_FLAG_MSI_REMAP in ITS driver. But this is not the case with
>> our ITS. 
> 
> The ITS itself is perfectly safe, as it does perform device isolation
> just fine (at least as far as I can tell from this bug description).
> 
> There is two things we need to take care of:
> - When the device is used on the host, the hardwired MSI region must be
> excluded from the DMA IOVA allocator, and the iommu_dma_map_msi_msg()
> call becomes a NOP.
> - When the device is assigned to VFIO, the MSI region must be exposed to
> userspace through /sys so that it knows that the guest RAM cannot alias
> with this region (or face the corruption we've talked about above).
> 
> None of that actually involves the ITS. Eric's stuff has some of the
> initial infrastructure, but there is of course more to it. I'll let
> Robin chime in and correct me if I've missed something (very likely).

That's pretty much it. Once Eric's patches for the iommu_resv_regions
interface have been merged, I'm planning to convert IOMMU-DMA over to
using those instead of the internal PCI-specific hook it currently has.
Then we would simply need the SMMU driver to expose this hardwired MSI
region where necessary so that IOMMU-DMA can directly insert 1:1
iommu_dma_msi_page entries to cover it up-front in iommu_dma_init_domain().

Robin.

> 
> Thanks,
> 
> 	M.
> 

Powered by blists - more mailing lists