Message-ID: <1f1de259-d7e1-8767-2e4b-dfc7e342ef82@oracle.com>
Date:   Fri, 13 Jan 2017 08:59:01 -0700
From:   Khalid Aziz <khalid.aziz@...cle.com>
To:     Rob Gardner <rob.gardner@...cle.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>, davem@...emloft.net,
        corbet@....net, arnd@...db.de, akpm@...ux-foundation.org
Cc:     hpa@...or.com, viro@...iv.linux.org.uk, nitin.m.gupta@...cle.com,
        chris.hyser@...cle.com, tushar.n.dave@...cle.com,
        sowmini.varadhan@...cle.com, mike.kravetz@...cle.com,
        adam.buchbinder@...il.com, minchan@...nel.org, hughd@...gle.com,
        kirill.shutemov@...ux.intel.com, keescook@...omium.org,
        allen.pais@...cle.com, aryabinin@...tuozzo.com,
        atish.patra@...cle.com, joe@...ches.com, pmladek@...e.com,
        jslaby@...e.cz, cmetcalf@...lanox.com,
        paul.gortmaker@...driver.com, mhocko@...e.com, jmarchan@...hat.com,
        lstoakes@...il.com, 0x7f454c46@...il.com, vbabka@...e.cz,
        tglx@...utronix.de, mingo@...hat.com, dan.j.williams@...el.com,
        iamjoonsoo.kim@....com, mgorman@...hsingularity.net,
        vdavydov.dev@...il.com, hannes@...xchg.org, namit@...are.com,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        sparclinux@...r.kernel.org, linux-arch@...r.kernel.org,
        x86@...nel.org, linux-mm@...ck.org
Subject: Re: [PATCH v4 0/4] Application Data Integrity feature introduced by
 SPARC M7

On 01/13/2017 08:29 AM, Rob Gardner wrote:
> On 01/13/2017 07:48 AM, Khalid Aziz wrote:
>> On 01/12/2017 06:31 PM, Rob Gardner wrote:
>>> On 01/12/2017 05:22 PM, Khalid Aziz wrote:
>>>> On 01/12/2017 10:53 AM, Dave Hansen wrote:
>>>>> On 01/12/2017 08:50 AM, Khalid Aziz wrote:
>>>>>> 2. Any shared page that has ADI protection enabled on it, must
>>>>>> stay ADI
>>>>>> protected across all processes sharing it.
>>>>>
>>>>> Is that true?
>>>>>
>>>>> What happens if a page with ADI tags set is accessed via a PTE without
>>>>> the ADI enablement bit set?
>>>>
>>>> ADI protection applies across all processes in terms of all of them
>>>> must use the same tag to access the shared memory, but if a process
>>>> accesses a shared page with TTE.mcde bit cleared, access will be
>>>> granted.
>>>>
>>>>>
>>>>>> COW creates an intersection of the two. It creates a new copy of the
>>>>>> shared data. It is a new data page and hence the process creating it
>>>>>> must be the one responsible for enabling ADI protection on it.
>>>>>
>>>>> Do you mean that the application must be responsible?  Or the kernel
>>>>> running in the context of the new process must be responsible?
>>>>>
>>>>>> It is also a copy of what was ADI protected data, so should it
>>>>>> inherit the protection instead?
>>>>>
>>>>> I think the COW'd copy must inherit the VMA bit, the PTE bits, and the
>>>>> tags on the cachelines.
>>>>>
>>>>>> I misspoke earlier. I had misinterpreted the results of test I ran.
>>>>>> Changing the tag on shared memory is allowed by memory controller.
>>>>>> The
>>>>>> requirement is every one sharing the page must switch to the new
>>>>>> tag or
>>>>>> else they get SIGSEGV.
>>>>>
>>>>> I asked this in the last mail, but I guess I'll ask it again. Please
>>>>> answer this directly.
>>>>>
>>>>> If we require that everyone coordinate their tags on the backing
>>>>> physical memory, and we allow a lower-privileged program to access the
>>>>> same data as a more-privileged one, then the lower-privilege app can
>>>>> cause arbitrary crashes in the privileged application.
>>>>>
>>>>> For instance, say sudo mmap()'s /etc/passwd and uses ADI tags to
>>>>> protect
>>>>> the mapping.  Couldn't any other app in the system prevent sudo from
>>>>> working?
>>>>>
>>>>> How can we *EVER* allow tags to be set on non-writable mappings?
>>>
>>> I don't think you can write a tag to memory if you don't have write
>>> access in the TTE. Writing a tag requires a store instruction, and if
>>> the machine is at all sane, this will fault if you don't have write
>>> access.
>>>
>>
>> But could you have mmap'd the file writable, set the tags and then
>> changed the protection on memory to read-only? That would be the
>> logical way to ADI protect a memory being used to mmap a file. Right?
>
>
> Sure, if you have write access to begin with, you can set memory
> versions, then remove write access to the page. But I think the point is
> that if a process doesn't have write access, and cannot get it, then it
> will not ever be able to change memory versions. So in the example of a
> non-root process opening /etc/passwd (read only), and mmaping it, the
> mapping would be read-only as well. Personally I don't really see a use
> case for ADI on memory mapped to a file. In an abstract sense, the
> "backing store" for a memory mapped file is the file itself on disk, not
> physical memory. And there is already a way to restrict access to files,
> so perhaps ADI should simply be disallowed for memory mapped to files,
> and this particular complication can be avoided. Thoughts?

Hi Rob,

That is a good way to look at it. Memory mapped files already have a 
protection mechanism in place.

>
> Incidentally, I see ADI as primarily a way to protect memory from
> improper access within a process or group of cooperating processes.
> There is already a way to protect memory from unrelated processes, and
> if that is circumvented somehow, then ADI won't help at all. Perhaps we
> should stop talking about ADI as a "security" feature; It does add a
> layer of protection against buffer overflow attacks, but this attack
> only exists when there is a bug in the underlying application. If an
> attacker gains access to the virtual memory for a process, then nothing
> can help you.
>

That does make sense. Looking at ADI as a mechanism to prevent 
unintended improper access to memory through buffer overflow or other 
mechanism, would it still make sense to support ADI tags on mmap'd files 
within the group of cooperating processes? Say we have a process that 
mmap's a large file and then forks off a bunch of children that process 
smaller segments of that file. We would want to make sure these children 
do not step over each other's segments of the file due to programming 
flaw or compromise. Parent process could tag each segment with a 
different tag and give one tag to each child process.

I want to be sure we are not shutting down potential useful applications 
of ADI before we choose to not support ADI with memory mapped files.

I appreciate your input.

Thanks,
Khalid

>
> Rob
>
>
>>
>> --
>> Khalid
>>
>>> Rob
>>>
>>>
>>>
>>>>
>>>> I understand your quetion better now. That is a very valid concern.
>>>> Using ADI tags to prevent an unauthorized process from just reading
>>>> data in memory, say an in-memory copy of database, is one of the use
>>>> cases for ADI. This means there is a reasonable case to allow enabling
>>>> ADI and setting tags even on non-writable mappings. On the other hand,
>>>> if an unauthorized process manages to map the right memory pages in
>>>> its address space, it can read them any way by not setting TTE.mcd.
>>>>
>>>> Userspace app can set tag on any memory it has mapped in without
>>>> requiring assistance from kernel. Can this problem be solved by not
>>>> allowing setting TTE.mcd on non-writable mappings? Doesn't the same
>>>> problem occur on writable mappings? If a privileged process mmap()'s a
>>>> writable file with MAP_SHARED, enables ADI and sets tag on the mmap'd
>>>> memory region, then another lower privilege process mmap's the same
>>>> file writable (assuming file permissions allow it to), enables ADI and
>>>> sets a different tag on it, the privileged process would get SIGSEGV
>>>> when it tries to access the mmap'd file. Right?
>>>
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe sparclinux" in
>>> the body of a message to majordomo@...r.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe sparclinux" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists