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

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:	Mon, 7 Mar 2016 12:44:49 -0700
From:	Khalid Aziz <khalid.aziz@...cle.com>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	David Miller <davem@...emloft.net>,
	Jonathan Corbet <corbet@....net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	dingel@...ux.vnet.ibm.com, bob.picco@...cle.com,
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Arnd Bergmann <arnd@...db.de>, sparclinux@...r.kernel.org,
	Rob Gardner <rob.gardner@...cle.com>,
	Michal Hocko <mhocko@...e.cz>, chris.hyser@...cle.com,
	Richard Weinberger <richard@....at>,
	Vlastimil Babka <vbabka@...e.cz>,
	Konstantin Khlebnikov <koct9i@...il.com>,
	Oleg Nesterov <oleg@...hat.com>,
	Greg Thelen <gthelen@...gle.com>, Jan Kara <jack@...e.cz>,
	xiexiuqi@...wei.com, Vineet.Gupta1@...opsys.com,
	Andrew Lutomirski <luto@...nel.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Benjamin Segall <bsegall@...gle.com>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Davidlohr Bueso <dave@...olabs.net>,
	Alexey Dobriyan <adobriyan@...il.com>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	linux-arch <linux-arch@...r.kernel.org>,
	Linux API <linux-api@...r.kernel.org>
Subject: Re: [PATCH v2] sparc64: Add support for Application Data Integrity
 (ADI)

On 03/07/2016 11:49 AM, Andy Lutomirski wrote:
> On Mon, Mar 7, 2016 at 10:22 AM, Khalid Aziz <khalid.aziz@...cle.com> wrote:
>> No, it changes the tag associated with the virtual address for the caller.
>> Physical page backing this virtual address is unaffected. Tag checking is
>> done for virtual addresses. The one restriction where physical address is
>> relevant is when two processes map the same physical page, they both have to
>> use the same tag for the virtual addresses that map on to the shared
>> physical pages.
>
> Slow down, please.  *Why* do the tags for two different VAs that map
> to the same PA have to match?  What goes wrong if they don't, and why
> is requiring them to be the same a good idea?
>

Consider this scenario:

1. Process A creates a shm and attaches to it.
2. Process A fills shm with data it wants to share with only known 
processes. It enables ADI and sets tags on the shm.
3. Hacker triggers something like stack overflow on process A, exec's a 
new rogue binary and manages to attach to this shm. MMU knows tags were 
set on the virtual address mapping to the physical pages hosting the 
shm. If MMU does not require the rogue process to set the exact same 
tags on its mapping of the same shm, rogue process has defeated the ADI 
protection easily.

Does this make sense?

>>
>>>
>>> I sense DoS issues in your future.
>>>
>>
>> Are you concerned about DoS even if the tag is associated with virtual
>> address, not physical address?
>
> Yes, absolutely.
>
> fd = open("/lib/ld.so");
> mmap(fd)
> stxa to write the tag
>
> *boom*, presumably, because the tags apparently have to match for all mappings.
>

A process can not just write version tags and make the file inaccessible 
to others. It takes three steps to enable ADI:

1. Set PSTATE.mcde for the process.
2. Set TTE.mcd on all PTEs for the virtual addresses ADI is being 
enabled on.
3. Set version tags.

Unless all three steps are taken, tag checking will not be done. stxa 
will fail unless step 2 is completed. In your example, the step of 
setting TTE.mcd will force sharing to stop for the process through 
change_protection(), right?

Thanks for asking these tough questions. These are very helpful in 
refining my implementation and avoiding silly bugs.

--
Khalid