linux-kernel - Re: [PATCH] iommu/amd: Reserve exclusion range in iova-domain

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:   Fri, 29 Mar 2019 15:06:41 +0000
From:   Gary R Hook <ghook@....com>
To:     Joerg Roedel <joro@...tes.org>
CC:     "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        Joerg Roedel <jroedel@...e.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] iommu/amd: Reserve exclusion range in iova-domain

On 3/29/19 9:51 AM, Joerg Roedel wrote:
> Hi Gary,
> 
> On Thu, Mar 28, 2019 at 02:52:19PM +0000, Gary R Hook wrote:
>> On 3/28/19 5:44 AM, Joerg Roedel wrote:
>>> +		if (entry->prot & (1 << 2))
>>
>> Could we add #define IOMMU_WRITE_EXCL (1 << 2) to amd_iommu_types.h?
> 
> Yes, I replace that magic number with a descriptive name.

Super; thanks.

>> The problem I see here is that if, for some untold reason, there is more
>> than one exclusion range emitted, where only the last one wins in the
>> hardware, we'd still end up with more than one range reserved in the
>> IOVA tree. Clearly, this is extremely unlikely, but wouldn't we want to
>> protect against that sort of misuse/mistake?
>>
>> I could be missing something.
> 
> No, you are not, this could still be a problem. Until now it isn't,
> because this week was the first time I have every seen an AMD IOMMU
> system making use of exclusion ranges, and it doesn't have this problem.
> 
> But this problem has been in the code even before this patch and needs
> to be addressed separatly. I think it is the best option to cancel IOMMU
> initialization when the IVRS table defines conflicting exclusion ranges
> for a single IOMMU instance.

Really? Interesting. May I ask who, as I've not seen it yet, either?

This change accomplishes the goal of setting exclusion + reserving the 
IOVA range, and is verified with testing? Cool. One wonders if they've 
considered a unity range?

Agree that addressing the uniqueness issue separately is fine. I'd 
probably prefer "first one wins" until IOVA tree clean-up could be 
implemented for "last one wins".... but I'm seeing that there are at 
least two spots in the code that need attention. I may go experiment 
with this.

All told, this is really about handling a corner case, as the likelihood 
of a BIOS trying to set up >1 exclusion ranges seems improbable, if not 
impossible.

grh