linux-kernel - Re: [PATCH] iommu/iova: Sort out rbtree limit

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, 16 May 2017 08:42:21 -0500 (CDT)
From:   Aaron Sierra <asierra@...-inc.com>
To:     Robin Murphy <robin.murphy@....com>
Cc:     Joerg Roedel <joro@...tes.org>,
        iommu <iommu@...ts.linux-foundation.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Nate Watterson <nwatters@...eaurora.org>
Subject: Re: [PATCH] iommu/iova: Sort out rbtree limit_pfn handling

----- Original Message -----
> From: "Robin Murphy" <robin.murphy@....com>
> Sent: Tuesday, May 16, 2017 6:26:48 AM

> When walking the rbtree, the fact that iovad->start_pfn and limit_pfn
> are both inclusive limits creates an ambiguity once limit_pfn reaches
> the bottom of the address space and they overlap. Commit 5016bdb796b3
> ("iommu/iova: Fix underflow bug in __alloc_and_insert_iova_range") fixed
> the worst side-effect of this, that of underflow wraparound leading to
> bogus allocations, but the remaining fallout is that any attempt to
> allocate start_pfn itself erroneously fails.
> 
> The cleanest way to resolve the ambiguity is to simply make limit_pfn an
> exclusive limit when inside the guts of the rbtree. Since we're working
> with PFNs, representing one past the top of the address space is always
> possible without fear of overflow, and elsewhere it just makes life a
> little more straightforward.
> 
> Reported-by: Aaron Sierra <asierra@...-inc.com>
> Signed-off-by: Robin Murphy <robin.murphy@....com>
> ---
> 
> I've now run this through some more targeted testing, and I'm
> confident that it works as intended - Aaron, can you confirm if
> this satisfies your tests as well?

Robin,
Thanks for giving this issue some consideration. I can confirm that
your patch passes all of the test cases where I'd previously observed
allocation failures.

FWIW, my testing consists of defining a fixed limit_pfn (0xfffff) and
iterating over domains with start_pfn values of 0, then all powers-of-two
up to half of limit_pfn (0x80000).

For each domain, I set a fixed allocation unit size, calculate how many
allocations I expect to succeed, alloc and save iova structs until
allocation fails, then compare expected to actual. I do this for
allocation unit sizes of 1, 2, 4, 8, 50% of alloc-able range, and 100%
of alloc-able range.

-Aaron S.