linux-kernel - Re: [PATCH v5 4/4] sparc64: Add support for ADI (Application Data Integrity)

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:   Wed, 25 Jan 2017 15:50:36 -0700
From:   Rob Gardner <rob.gardner@...cle.com>
To:     Khalid Aziz <khalid.aziz@...cle.com>, davem@...emloft.net,
        corbet@....net
Cc:     viro@...iv.linux.org.uk, nitin.m.gupta@...cle.com,
        mike.kravetz@...cle.com, akpm@...ux-foundation.org,
        mingo@...nel.org, kirill.shutemov@...ux.intel.com,
        adam.buchbinder@...il.com, hughd@...gle.com, minchan@...nel.org,
        keescook@...omium.org, chris.hyser@...cle.com,
        atish.patra@...cle.com, cmetcalf@...lanox.com, atomlin@...hat.com,
        jslaby@...e.cz, joe@...ches.com, paul.gortmaker@...driver.com,
        mhocko@...e.com, lstoakes@...il.com, jack@...e.cz,
        dave.hansen@...ux.intel.com, vbabka@...e.cz,
        dan.j.williams@...el.com, iamjoonsoo.kim@....com,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        sparclinux@...r.kernel.org, linux-mm@...ck.org,
        Khalid Aziz <khalid@...ehiking.org>
Subject: Re: [PATCH v5 4/4] sparc64: Add support for ADI (Application Data
 Integrity)

On 01/25/2017 03:20 PM, Khalid Aziz wrote:
> On 01/25/2017 03:00 PM, Rob Gardner wrote:
>> On 01/25/2017 12:57 PM, Khalid Aziz wrote:
>>>
>>> @@ -157,6 +158,24 @@ int __get_user_pages_fast(unsigned long start,
>>> int nr_pages, int write,
>>>       pgd_t *pgdp;
>>>       int nr = 0;
>>>   +#ifdef CONFIG_SPARC64
>>> +    if (adi_capable()) {
>>> +        long addr = start;
>>> +
>>> +        /* If userspace has passed a versioned address, kernel
>>> +         * will not find it in the VMAs since it does not store
>>> +         * the version tags in the list of VMAs. Storing version
>>> +         * tags in list of VMAs is impractical since they can be
>>> +         * changed any time from userspace without dropping into
>>> +         * kernel. Any address search in VMAs will be done with
>>> +         * non-versioned addresses. Ensure the ADI version bits
>>> +         * are dropped here by sign extending the last bit before
>>> +         * ADI bits. IOMMU does not implement version tags.
>>> +         */
>>> +        addr = (addr << (long)adi_nbits()) >> (long)adi_nbits();
>>
>>
>> So you are depending on the sign extension to clear the ADI bits... but
>> this only happens if there is a zero in that "last bit before ADI bits".
>> If the last bit is a 1, then the ADI bits will be set instead of
>> cleared.  That seems like an unintended consequence given the comment. I
>> am aware of the value of adi_nbits() and of the number of valid bits in
>> a virtual address on the M7 processor, but wouldn't using 'unsigned
>> long' for everything here guarantee the ADI bits get cleared regardless
>> of the state of the last non-adi bit?
>
> Sign extension is the right thing to do. MMU considers values of 0 and 
> 15 for bits 63-60 to be untagged addresses and expects bit 59 to be 
> sign-extended for untagged virtual addresses. The code I added is 
> explicitly meant to sign-extend, not zero out the top 4 bits.

OK, that wasn't perfectly clear from the comment, which said "version 
bits are dropped".

So sign extending will produce an address that the MMU can use, but will 
it produce an address that will allow a successful search in the page 
tables? ie, was this same sign extending done when first handing out 
that virtual address to the user?

Rob