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

[Khalid Aziz]

On 03/08/2016 01:27 PM, David Miller wrote:

From: Khalid Aziz 
Date: Tue, 8 Mar 2016 13:16:11 -0700


On 03/08/2016 12:57 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:06:43 -0700


Good questions. Isn't set of valid VAs already constrained by VA_BITS
(set to 44 in arch/sparc/include/asm/processor_64.h)? As I see it we
are already not using the top 4 bits. Please correct me if I am wrong.


Another limiting constraint is the number of address bits coverable by
the 4-level page tables we use.  And this is sign extended so we have
a top-half and a bottom-half with a "hole" in the center of the VA
space.

I want some clarification on the top bits during ADI accesses.

If ADI is enabled, then the top bits of the virtual address are
intepreted as tag bits.  Once "verified" with the ADI settings, what
happense to these tag bits?  Are they dropped from the virtual address
before being passed down the TLB et al. for translations?


Bits 63-60 (tag bits) are dropped from the virtual address before
being passed down the TLB for translation when PSTATE.mcde = 1.


Ok and you said that values 15 and 0 are special.

I'm just wondering if this means you can't really use ADI mappings in
the top half of the 64-bit address space.  If the bits are dropped, they
will be zero, but they need to be all 1's for the top-half of the VA
space since it's sign extended.



According to the manual when PSTATE.mcde=1, bits 63:60 of the virtual 
address of any load or store (using virtual address) are masked before 
being sent to memory system which includes MMU. Hardware TSB walker 
masks bits 63:60 and then sign extends from bit 59 before generating TSB 
pointer and before comparison to TSB TTE VAs but the virtual address in 
the TTE tag that is written to DTLB is masked and not sign extended. 
Manual also states that for implementations that fully support 60 bits 
or more of virtual address, they must sign-extend virtual address in TSB 
TTE tag.


--
Khalid

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

[Khalid Aziz]

On 03/08/2016 01:27 PM, David Miller wrote:

From: Khalid Aziz 
Date: Tue, 8 Mar 2016 13:16:11 -0700


On 03/08/2016 12:57 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:06:43 -0700


Good questions. Isn't set of valid VAs already constrained by VA_BITS
(set to 44 in arch/sparc/include/asm/processor_64.h)? As I see it we
are already not using the top 4 bits. Please correct me if I am wrong.


Another limiting constraint is the number of address bits coverable by
the 4-level page tables we use.  And this is sign extended so we have
a top-half and a bottom-half with a "hole" in the center of the VA
space.

I want some clarification on the top bits during ADI accesses.

If ADI is enabled, then the top bits of the virtual address are
intepreted as tag bits.  Once "verified" with the ADI settings, what
happense to these tag bits?  Are they dropped from the virtual address
before being passed down the TLB et al. for translations?


Bits 63-60 (tag bits) are dropped from the virtual address before
being passed down the TLB for translation when PSTATE.mcde = 1.


Ok and you said that values 15 and 0 are special.

I'm just wondering if this means you can't really use ADI mappings in
the top half of the 64-bit address space.  If the bits are dropped, they
will be zero, but they need to be all 1's for the top-half of the VA
space since it's sign extended.



According to the manual when PSTATE.mcde=1, bits 63:60 of the virtual 
address of any load or store (using virtual address) are masked before 
being sent to memory system which includes MMU. Hardware TSB walker 
masks bits 63:60 and then sign extends from bit 59 before generating TSB 
pointer and before comparison to TSB TTE VAs but the virtual address in 
the TTE tag that is written to DTLB is masked and not sign extended. 
Manual also states that for implementations that fully support 60 bits 
or more of virtual address, they must sign-extend virtual address in TSB 
TTE tag.


--
Khalid

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

[David Miller]

From: Khalid Aziz 
Date: Tue, 8 Mar 2016 13:16:11 -0700

> On 03/08/2016 12:57 PM, David Miller wrote:
>> From: Khalid Aziz 
>> Date: Mon, 7 Mar 2016 14:06:43 -0700
>>
>>> Good questions. Isn't set of valid VAs already constrained by VA_BITS
>>> (set to 44 in arch/sparc/include/asm/processor_64.h)? As I see it we
>>> are already not using the top 4 bits. Please correct me if I am wrong.
>>
>> Another limiting constraint is the number of address bits coverable by
>> the 4-level page tables we use.  And this is sign extended so we have
>> a top-half and a bottom-half with a "hole" in the center of the VA
>> space.
>>
>> I want some clarification on the top bits during ADI accesses.
>>
>> If ADI is enabled, then the top bits of the virtual address are
>> intepreted as tag bits.  Once "verified" with the ADI settings, what
>> happense to these tag bits?  Are they dropped from the virtual address
>> before being passed down the TLB et al. for translations?
> 
> Bits 63-60 (tag bits) are dropped from the virtual address before
> being passed down the TLB for translation when PSTATE.mcde = 1.

Ok and you said that values 15 and 0 are special.

I'm just wondering if this means you can't really use ADI mappings in
the top half of the 64-bit address space.  If the bits are dropped, they
will be zero, but they need to be all 1's for the top-half of the VA
space since it's sign extended.

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

[David Miller]

From: Khalid Aziz 
Date: Tue, 8 Mar 2016 13:16:11 -0700

> On 03/08/2016 12:57 PM, David Miller wrote:
>> From: Khalid Aziz 
>> Date: Mon, 7 Mar 2016 14:06:43 -0700
>>
>>> Good questions. Isn't set of valid VAs already constrained by VA_BITS
>>> (set to 44 in arch/sparc/include/asm/processor_64.h)? As I see it we
>>> are already not using the top 4 bits. Please correct me if I am wrong.
>>
>> Another limiting constraint is the number of address bits coverable by
>> the 4-level page tables we use.  And this is sign extended so we have
>> a top-half and a bottom-half with a "hole" in the center of the VA
>> space.
>>
>> I want some clarification on the top bits during ADI accesses.
>>
>> If ADI is enabled, then the top bits of the virtual address are
>> intepreted as tag bits.  Once "verified" with the ADI settings, what
>> happense to these tag bits?  Are they dropped from the virtual address
>> before being passed down the TLB et al. for translations?
> 
> Bits 63-60 (tag bits) are dropped from the virtual address before
> being passed down the TLB for translation when PSTATE.mcde = 1.

Ok and you said that values 15 and 0 are special.

I'm just wondering if this means you can't really use ADI mappings in
the top half of the 64-bit address space.  If the bits are dropped, they
will be zero, but they need to be all 1's for the top-half of the VA
space since it's sign extended.

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

[Khalid Aziz]

On 03/08/2016 12:57 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:06:43 -0700


Good questions. Isn't set of valid VAs already constrained by VA_BITS
(set to 44 in arch/sparc/include/asm/processor_64.h)? As I see it we
are already not using the top 4 bits. Please correct me if I am wrong.


Another limiting constraint is the number of address bits coverable by
the 4-level page tables we use.  And this is sign extended so we have
a top-half and a bottom-half with a "hole" in the center of the VA
space.

I want some clarification on the top bits during ADI accesses.

If ADI is enabled, then the top bits of the virtual address are
intepreted as tag bits.  Once "verified" with the ADI settings, what
happense to these tag bits?  Are they dropped from the virtual address
before being passed down the TLB et al. for translations?


Bits 63-60 (tag bits) are dropped from the virtual address before being 
passed down the TLB for translation when PSTATE.mcde = 1.


--
Khalid



If not, then this means you have to map ADI memory to the correct
location so that the tags match up.

And if that's the case, if you really wanted to mix tags within a
single page, you'd have to map that page several times, once for each
and every cacheline granular tag you'd like to use within that page.

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

[Khalid Aziz]

On 03/08/2016 12:57 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:06:43 -0700


Good questions. Isn't set of valid VAs already constrained by VA_BITS
(set to 44 in arch/sparc/include/asm/processor_64.h)? As I see it we
are already not using the top 4 bits. Please correct me if I am wrong.


Another limiting constraint is the number of address bits coverable by
the 4-level page tables we use.  And this is sign extended so we have
a top-half and a bottom-half with a "hole" in the center of the VA
space.

I want some clarification on the top bits during ADI accesses.

If ADI is enabled, then the top bits of the virtual address are
intepreted as tag bits.  Once "verified" with the ADI settings, what
happense to these tag bits?  Are they dropped from the virtual address
before being passed down the TLB et al. for translations?


Bits 63-60 (tag bits) are dropped from the virtual address before being 
passed down the TLB for translation when PSTATE.mcde = 1.


--
Khalid



If not, then this means you have to map ADI memory to the correct
location so that the tags match up.

And if that's the case, if you really wanted to mix tags within a
single page, you'd have to map that page several times, once for each
and every cacheline granular tag you'd like to use within that page.

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:06:43 -0700

> Good questions. Isn't set of valid VAs already constrained by VA_BITS
> (set to 44 in arch/sparc/include/asm/processor_64.h)? As I see it we
> are already not using the top 4 bits. Please correct me if I am wrong.

Another limiting constraint is the number of address bits coverable by
the 4-level page tables we use.  And this is sign extended so we have
a top-half and a bottom-half with a "hole" in the center of the VA
space.

I want some clarification on the top bits during ADI accesses.

If ADI is enabled, then the top bits of the virtual address are
intepreted as tag bits.  Once "verified" with the ADI settings, what
happense to these tag bits?  Are they dropped from the virtual address
before being passed down the TLB et al. for translations?

If not, then this means you have to map ADI memory to the correct
location so that the tags match up.

And if that's the case, if you really wanted to mix tags within a
single page, you'd have to map that page several times, once for each
and every cacheline granular tag you'd like to use within that page.

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:06:43 -0700

> Good questions. Isn't set of valid VAs already constrained by VA_BITS
> (set to 44 in arch/sparc/include/asm/processor_64.h)? As I see it we
> are already not using the top 4 bits. Please correct me if I am wrong.

Another limiting constraint is the number of address bits coverable by
the 4-level page tables we use.  And this is sign extended so we have
a top-half and a bottom-half with a "hole" in the center of the VA
space.

I want some clarification on the top bits during ADI accesses.

If ADI is enabled, then the top bits of the virtual address are
intepreted as tag bits.  Once "verified" with the ADI settings, what
happense to these tag bits?  Are they dropped from the virtual address
before being passed down the TLB et al. for translations?

If not, then this means you have to map ADI memory to the correct
location so that the tags match up.

And if that's the case, if you really wanted to mix tags within a
single page, you'd have to map that page several times, once for each
and every cacheline granular tag you'd like to use within that page.

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

[James Morris]

On 03/08/2016 10:48 AM, James Morris wrote:

On 03/08/2016 06:54 AM, Andy Lutomirski wrote:


This makes sense, but I still think the design is poor.  If the hacker
gets code execution, then they can trivially brute force the ADI bits.



ADI in this scenario is intended to prevent the attacker from gaining
code execution in the first place.


Here's some more background from Enrico Perla (who literally wrote the 
book on kernel exploitation):


https://blogs.oracle.com/enrico/entry/hardening_allocators_with_adi

Probably the most significant advantage from a security point of view is 
the ability to eliminate an entire class of vulnerability: adjacent heap 
overflows, as discussed above, where, for example, adjacent heap objects 
are tagged differently.  Classic linear buffer overflows can be eliminated.


As Kees Cook outlined at the 2015 kernel summit, it's best to mitigate 
classes of vulnerabilities rather than patch each instance:


https://outflux.net/slides/2011/defcon/kernel-exploitation.pdf

The Linux ADI implementation is currently very rudimentary, and we 
definitely welcome continued feedback from the community and ideas as it 
evolves.


- James

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

[James Morris]

On 03/08/2016 10:48 AM, James Morris wrote:

On 03/08/2016 06:54 AM, Andy Lutomirski wrote:


This makes sense, but I still think the design is poor.  If the hacker
gets code execution, then they can trivially brute force the ADI bits.



ADI in this scenario is intended to prevent the attacker from gaining
code execution in the first place.


Here's some more background from Enrico Perla (who literally wrote the 
book on kernel exploitation):


https://blogs.oracle.com/enrico/entry/hardening_allocators_with_adi

Probably the most significant advantage from a security point of view is 
the ability to eliminate an entire class of vulnerability: adjacent heap 
overflows, as discussed above, where, for example, adjacent heap objects 
are tagged differently.  Classic linear buffer overflows can be eliminated.


As Kees Cook outlined at the 2015 kernel summit, it's best to mitigate 
classes of vulnerabilities rather than patch each instance:


https://outflux.net/slides/2011/defcon/kernel-exploitation.pdf

The Linux ADI implementation is currently very rudimentary, and we 
definitely welcome continued feedback from the community and ideas as it 
evolves.


- James

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 17:21:05 -0700

> Can we enable ADI support for swappable pages in a subsequent update
> after the core functionality is stable on mlock'd pages?

I already said no.

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 17:21:05 -0700

> Can we enable ADI support for swappable pages in a subsequent update
> after the core functionality is stable on mlock'd pages?

I already said no.

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

[David Miller]

From: Rob Gardner 
Date: Mon, 7 Mar 2016 15:13:31 -0800

> You can easily read ADI tags with a simple ldxa #ASI_MCD_PRIMARY
> instruction.

Awesome!

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

[David Miller]

From: Rob Gardner 
Date: Mon, 7 Mar 2016 15:13:31 -0800

> You can easily read ADI tags with a simple ldxa #ASI_MCD_PRIMARY
> instruction.

Awesome!

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

[Rob Gardner]

On 03/07/2016 10:39 AM, Khalid Aziz wrote:

On 03/07/2016 11:12 AM, Dave Hansen wrote:

On 03/07/2016 09:53 AM, Andy Lutomirski wrote:

Also, what am I missing?  Tying these tags to the physical page seems
like a poor design to me.  This seems really awkward to use.


Yeah, can you describe the structures that store these things? Surely
the hardware has some kind of lookup tables for them and stores them in
memory _somewhere_.



Version tags are tied to virtual addresses, not physical pages.

Where exactly are the tags stored is part of processor architecture 
and I am not privy to that. MMU stores these lookup tables somewhere 
and uses it to authenticate access to virtual addresses. It really is 
irrelevant to kernel how MMU implements access controls as long as we 
have access to the knowledge of how to use it.


The tags are stored in physical memory, and you can write a tag directly 
to that memory via stxa with ASI_MCD_REAL and completely bypass the MMU. 
When you do that, the tag will still be seen by any virtual address that 
maps to that physical address.


Rob

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

[Rob Gardner]

On 03/07/2016 10:39 AM, Khalid Aziz wrote:

On 03/07/2016 11:12 AM, Dave Hansen wrote:

On 03/07/2016 09:53 AM, Andy Lutomirski wrote:

Also, what am I missing?  Tying these tags to the physical page seems
like a poor design to me.  This seems really awkward to use.


Yeah, can you describe the structures that store these things? Surely
the hardware has some kind of lookup tables for them and stores them in
memory _somewhere_.



Version tags are tied to virtual addresses, not physical pages.

Where exactly are the tags stored is part of processor architecture 
and I am not privy to that. MMU stores these lookup tables somewhere 
and uses it to authenticate access to virtual addresses. It really is 
irrelevant to kernel how MMU implements access controls as long as we 
have access to the knowledge of how to use it.


The tags are stored in physical memory, and you can write a tag directly 
to that memory via stxa with ASI_MCD_REAL and completely bypass the MMU. 
When you do that, the tag will still be seen by any virtual address that 
maps to that physical address.


Rob

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

[Khalid Aziz]

On 03/07/2016 12:16 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:24:54 -0700


Tags can be cleared by user by setting tag to 0. Tags are
automatically cleared by the hardware when the mapping for a virtual
address is removed from TSB (which is why swappable pages are a
problem), so kernel does not have to do it as part of clean up.


You might be able to crib some bits for the Tag in the swp_entry_t, it's
64-bit and you can therefore steal bits from the offset field.

That way you'll have the ADI tag in the page tables, ready to re-install
at swapin time.



Hi Dave,

Can we enable ADI support for swappable pages in a subsequent update 
after the core functionality is stable on mlock'd pages?


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 12:16 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:24:54 -0700


Tags can be cleared by user by setting tag to 0. Tags are
automatically cleared by the hardware when the mapping for a virtual
address is removed from TSB (which is why swappable pages are a
problem), so kernel does not have to do it as part of clean up.


You might be able to crib some bits for the Tag in the swp_entry_t, it's
64-bit and you can therefore steal bits from the offset field.

That way you'll have the ADI tag in the page tables, ready to re-install
at swapin time.



Hi Dave,

Can we enable ADI support for swappable pages in a subsequent update 
after the core functionality is stable on mlock'd pages?


Thanks,
Khalid

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

[James Morris]

On 03/08/2016 06:54 AM, Andy Lutomirski wrote:


This makes sense, but I still think the design is poor.  If the hacker
gets code execution, then they can trivially brute force the ADI bits.



ADI in this scenario is intended to prevent the attacker from gaining 
code execution in the first place.

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

[James Morris]

On 03/08/2016 06:54 AM, Andy Lutomirski wrote:


This makes sense, but I still think the design is poor.  If the hacker
gets code execution, then they can trivially brute force the ADI bits.



ADI in this scenario is intended to prevent the attacker from gaining 
code execution in the first place.

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

[James Morris]

On 03/08/2016 07:58 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 13:41:39 -0700


Shared data may not always be backed by a file. My understanding is
one of the use cases is for in-memory databases. This shared space
could also be used to hand off transactions in flight to other
processes. These transactions in flight would not be backed by a
file. Some of these use cases might not use shmfs even. Setting ADI
bits at virtual address level catches all these cases since what backs
the tagged virtual address can be anything - a mapped file, mmio
space, just plain chunk of memory.


Frankly the most interesting use case to me is simply finding bugs
and memory scribbles, and for that we're want to be able to ADI
arbitrary memory returned from malloc() and friends.

I personally see ADI more as a debugging than a security feature,
but that's just my view.


This is certainly a major use of the feature. The Solaris folks have 
made some interesting use of it here:


https://docs.oracle.com/cd/E37069_01/html/E37085/gphwb.html

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

[Rob Gardner]

On 03/07/2016 10:24 AM, Khalid Aziz wrote:


Tags can be cleared by user by setting tag to 0. Tags are 
automatically cleared by the hardware when the mapping for a virtual 
address is removed from TSB (which is why swappable pages are a 
problem), so kernel does not have to do it as part of clean up.




I don't understand this. The hardware isn't involved  when a mapping for 
a virtual address is removed from the TSB, so how could it automatically 
clear tags?


Rob

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

[Rob Gardner]

On 03/07/2016 10:24 AM, Khalid Aziz wrote:


Tags can be cleared by user by setting tag to 0. Tags are 
automatically cleared by the hardware when the mapping for a virtual 
address is removed from TSB (which is why swappable pages are a 
problem), so kernel does not have to do it as part of clean up.




I don't understand this. The hardware isn't involved  when a mapping for 
a virtual address is removed from the TSB, so how could it automatically 
clear tags?


Rob

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

[James Morris]

On 03/08/2016 07:58 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 13:41:39 -0700


Shared data may not always be backed by a file. My understanding is
one of the use cases is for in-memory databases. This shared space
could also be used to hand off transactions in flight to other
processes. These transactions in flight would not be backed by a
file. Some of these use cases might not use shmfs even. Setting ADI
bits at virtual address level catches all these cases since what backs
the tagged virtual address can be anything - a mapped file, mmio
space, just plain chunk of memory.


Frankly the most interesting use case to me is simply finding bugs
and memory scribbles, and for that we're want to be able to ADI
arbitrary memory returned from malloc() and friends.

I personally see ADI more as a debugging than a security feature,
but that's just my view.


This is certainly a major use of the feature. The Solaris folks have 
made some interesting use of it here:


https://docs.oracle.com/cd/E37069_01/html/E37085/gphwb.html

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

[Khalid Aziz]

On 03/07/2016 04:12 PM, Rob Gardner wrote:

On 03/07/2016 01:33 PM, Khalid Aziz wrote:


That is a possibility but limited in scope. An address range covered
by a single TTE can have large number of tags. Version tags are set on
cacheline. In extreme case, one could set a tag for each set of
64-bytes in a page. Also tags are set completely in userspace and no
transition occurs to kernel space, so kernel has no idea of what tags
have been set.


   ...

I have not found a way to query the MMU on tags.



To query the tag for a cache line, you just read it back with ldxa and
ASI_MCD_PRIMARY (ie, asi 0x90), basically the same way you stored the
tag in the first place.



Thanks, Rob. I just saw it while reading through the manual.

--
Khalid

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

[Khalid Aziz]

On 03/07/2016 04:12 PM, Rob Gardner wrote:

On 03/07/2016 01:33 PM, Khalid Aziz wrote:


That is a possibility but limited in scope. An address range covered
by a single TTE can have large number of tags. Version tags are set on
cacheline. In extreme case, one could set a tag for each set of
64-bytes in a page. Also tags are set completely in userspace and no
transition occurs to kernel space, so kernel has no idea of what tags
have been set.


   ...

I have not found a way to query the MMU on tags.



To query the tag for a cache line, you just read it back with ldxa and
ASI_MCD_PRIMARY (ie, asi 0x90), basically the same way you stored the
tag in the first place.



Thanks, Rob. I just saw it while reading through the manual.

--
Khalid

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

[Rob Gardner]

On 03/07/2016 01:38 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:33:56 -0700


On 03/07/2016 12:16 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:24:54 -0700


Tags can be cleared by user by setting tag to 0. Tags are
automatically cleared by the hardware when the mapping for a virtual
address is removed from TSB (which is why swappable pages are a
problem), so kernel does not have to do it as part of clean up.

You might be able to crib some bits for the Tag in the swp_entry_t,
it's
64-bit and you can therefore steal bits from the offset field.

That way you'll have the ADI tag in the page tables, ready to
re-install
at swapin time.


That is a possibility but limited in scope. An address range covered
by a single TTE can have large number of tags. Version tags are set on
cacheline. In extreme case, one could set a tag for each set of
64-bytes in a page. Also tags are set completely in userspace and no
transition occurs to kernel space, so kernel has no idea of what tags
have been set. I have not found a way to query the MMU on tags.

I will think some more about it.

That would mean that ADI is impossible to use for swappable memory.

...

If that's true I'm extremely disappointed that they devoted so much
silicon and engineering to this feature yet didn't take that one
critical step to make it generally useful. :(


You can easily read ADI tags with a simple ldxa #ASI_MCD_PRIMARY 
instruction.


Rob

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

[Rob Gardner]

On 03/07/2016 01:38 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:33:56 -0700


On 03/07/2016 12:16 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:24:54 -0700


Tags can be cleared by user by setting tag to 0. Tags are
automatically cleared by the hardware when the mapping for a virtual
address is removed from TSB (which is why swappable pages are a
problem), so kernel does not have to do it as part of clean up.

You might be able to crib some bits for the Tag in the swp_entry_t,
it's
64-bit and you can therefore steal bits from the offset field.

That way you'll have the ADI tag in the page tables, ready to
re-install
at swapin time.


That is a possibility but limited in scope. An address range covered
by a single TTE can have large number of tags. Version tags are set on
cacheline. In extreme case, one could set a tag for each set of
64-bytes in a page. Also tags are set completely in userspace and no
transition occurs to kernel space, so kernel has no idea of what tags
have been set. I have not found a way to query the MMU on tags.

I will think some more about it.

That would mean that ADI is impossible to use for swappable memory.

...

If that's true I'm extremely disappointed that they devoted so much
silicon and engineering to this feature yet didn't take that one
critical step to make it generally useful. :(


You can easily read ADI tags with a simple ldxa #ASI_MCD_PRIMARY 
instruction.


Rob

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

[Rob Gardner]

On 03/07/2016 01:33 PM, Khalid Aziz wrote:


That is a possibility but limited in scope. An address range covered 
by a single TTE can have large number of tags. Version tags are set on 
cacheline. In extreme case, one could set a tag for each set of 
64-bytes in a page. Also tags are set completely in userspace and no 
transition occurs to kernel space, so kernel has no idea of what tags 
have been set.


  ...

I have not found a way to query the MMU on tags.



To query the tag for a cache line, you just read it back with ldxa and 
ASI_MCD_PRIMARY (ie, asi 0x90), basically the same way you stored the 
tag in the first place.


Rob

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

[Rob Gardner]

On 03/07/2016 01:33 PM, Khalid Aziz wrote:


That is a possibility but limited in scope. An address range covered 
by a single TTE can have large number of tags. Version tags are set on 
cacheline. In extreme case, one could set a tag for each set of 
64-bytes in a page. Also tags are set completely in userspace and no 
transition occurs to kernel space, so kernel has no idea of what tags 
have been set.


  ...

I have not found a way to query the MMU on tags.



To query the tag for a cache line, you just read it back with ldxa and 
ASI_MCD_PRIMARY (ie, asi 0x90), basically the same way you stored the 
tag in the first place.


Rob

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

[Dave Hansen]

On 03/07/2016 11:46 AM, Khalid Aziz wrote:
> On 03/07/2016 12:22 PM, David Miller wrote:
>> Khalid, maybe you should share notes with the folks working on x86
>> protection keys.
> 
> Good idea. Sparc ADI feature is indeed similar to x86 protection keys
> sounds like.

There are definitely some similarities.  But protection keys doesn't
have any additional tables in which to keep metadata.  It keeps all of
its data in the page tables.  It also doesn't have an impact on the
virtual address layout.

But, it does have metadata to store in the VMA, has a special
siginfo->si_code, and it uses mprotect() (although a new pkey_mprotect()
variant that takes an extra argument).

Protection Keys are described a bit more here:

> http://git.kernel.org/cgit/linux/kernel/git/daveh/x86-pkeys.git/tree/Documentation/x86/protection-keys.txt?h=pkeys-v025=1b5b8a8836de8eb667027178b4820665dea5a038

MPX is another Intel feature separate from protection keys, but *it* has
some tables that it keep its metadata memory and special special
instructions to move metadata in and out of it.  It also has a prctl()
to enable/disable kernel assistance for the feature.  Unlike ADI, the
tables are exposed (and accessible) to user applications in normal
application memory.

MPX's documentation is here:

> http://git.kernel.org/cgit/linux/kernel/git/daveh/x86-pkeys.git/tree/Documentation/x86/intel_mpx.txt

Overall, I'm not seeing much overlap at all between the features, honestly.

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

[Dave Hansen]

On 03/07/2016 11:46 AM, Khalid Aziz wrote:
> On 03/07/2016 12:22 PM, David Miller wrote:
>> Khalid, maybe you should share notes with the folks working on x86
>> protection keys.
> 
> Good idea. Sparc ADI feature is indeed similar to x86 protection keys
> sounds like.

There are definitely some similarities.  But protection keys doesn't
have any additional tables in which to keep metadata.  It keeps all of
its data in the page tables.  It also doesn't have an impact on the
virtual address layout.

But, it does have metadata to store in the VMA, has a special
siginfo->si_code, and it uses mprotect() (although a new pkey_mprotect()
variant that takes an extra argument).

Protection Keys are described a bit more here:

> http://git.kernel.org/cgit/linux/kernel/git/daveh/x86-pkeys.git/tree/Documentation/x86/protection-keys.txt?h=pkeys-v025=1b5b8a8836de8eb667027178b4820665dea5a038

MPX is another Intel feature separate from protection keys, but *it* has
some tables that it keep its metadata memory and special special
instructions to move metadata in and out of it.  It also has a prctl()
to enable/disable kernel assistance for the feature.  Unlike ADI, the
tables are exposed (and accessible) to user applications in normal
application memory.

MPX's documentation is here:

> http://git.kernel.org/cgit/linux/kernel/git/daveh/x86-pkeys.git/tree/Documentation/x86/intel_mpx.txt

Overall, I'm not seeing much overlap at all between the features, honestly.

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

[Khalid Aziz]

On 03/07/2016 02:34 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:27:09 -0700


I agree with your point of view. PSTATE.mcde and TTE.mcd are set in
response to request from userspace. If userspace asked for them to be
set, they already know but it was the database guys that asked for
these two functions and they are the primary customers for the ADI
feature. I am not crazy about this idea since this extends the
mprotect API even further but would you consider using the return
value from mprotect to indicate if PSTATE.mcde or TTE.mcd were already
set on the given address?


Well, that's the idea.

If the mprotect using MAP_ADI or whatever succeeds, then ADI is
enabled.

Users can thus also pass MAP_ADI as a flag to mmap() to get ADI
protection from the very beginning.



MAP_ADI has been sitting in my backlog for some time. Looks like you 
just raised its priority ;)


--
Khalid

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

[Khalid Aziz]

On 03/07/2016 02:34 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:27:09 -0700


I agree with your point of view. PSTATE.mcde and TTE.mcd are set in
response to request from userspace. If userspace asked for them to be
set, they already know but it was the database guys that asked for
these two functions and they are the primary customers for the ADI
feature. I am not crazy about this idea since this extends the
mprotect API even further but would you consider using the return
value from mprotect to indicate if PSTATE.mcde or TTE.mcd were already
set on the given address?


Well, that's the idea.

If the mprotect using MAP_ADI or whatever succeeds, then ADI is
enabled.

Users can thus also pass MAP_ADI as a flag to mmap() to get ADI
protection from the very beginning.



MAP_ADI has been sitting in my backlog for some time. Looks like you 
just raised its priority ;)


--
Khalid

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:33:56 -0700

> On 03/07/2016 12:16 PM, David Miller wrote:
>> From: Khalid Aziz 
>> Date: Mon, 7 Mar 2016 11:24:54 -0700
>>
>>> Tags can be cleared by user by setting tag to 0. Tags are
>>> automatically cleared by the hardware when the mapping for a virtual
>>> address is removed from TSB (which is why swappable pages are a
>>> problem), so kernel does not have to do it as part of clean up.
>>
>> You might be able to crib some bits for the Tag in the swp_entry_t,
>> it's
>> 64-bit and you can therefore steal bits from the offset field.
>>
>> That way you'll have the ADI tag in the page tables, ready to
>> re-install
>> at swapin time.
>>
> 
> That is a possibility but limited in scope. An address range covered
> by a single TTE can have large number of tags. Version tags are set on
> cacheline. In extreme case, one could set a tag for each set of
> 64-bytes in a page. Also tags are set completely in userspace and no
> transition occurs to kernel space, so kernel has no idea of what tags
> have been set. I have not found a way to query the MMU on tags.
> 
> I will think some more about it.

That would mean that ADI is impossible to use for swappable memory.

...

If that's true I'm extremely disappointed that they devoted so much
silicon and engineering to this feature yet didn't take that one
critical step to make it generally useful. :(

We could have a way to do this via the kernel, wherein the user has a
contract with us.  Basically we have a call to pass the Tags (what
granularity to use for this is a design point, pages, cache lines,
etc.)  into the kernel and the user agrees not to change them behind
the kernel's back.

In return the kernel agrees to restore the tags upon swapin.

So we could support something for swappable pages, it would just be
more work.

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:33:56 -0700

> On 03/07/2016 12:16 PM, David Miller wrote:
>> From: Khalid Aziz 
>> Date: Mon, 7 Mar 2016 11:24:54 -0700
>>
>>> Tags can be cleared by user by setting tag to 0. Tags are
>>> automatically cleared by the hardware when the mapping for a virtual
>>> address is removed from TSB (which is why swappable pages are a
>>> problem), so kernel does not have to do it as part of clean up.
>>
>> You might be able to crib some bits for the Tag in the swp_entry_t,
>> it's
>> 64-bit and you can therefore steal bits from the offset field.
>>
>> That way you'll have the ADI tag in the page tables, ready to
>> re-install
>> at swapin time.
>>
> 
> That is a possibility but limited in scope. An address range covered
> by a single TTE can have large number of tags. Version tags are set on
> cacheline. In extreme case, one could set a tag for each set of
> 64-bytes in a page. Also tags are set completely in userspace and no
> transition occurs to kernel space, so kernel has no idea of what tags
> have been set. I have not found a way to query the MMU on tags.
> 
> I will think some more about it.

That would mean that ADI is impossible to use for swappable memory.

...

If that's true I'm extremely disappointed that they devoted so much
silicon and engineering to this feature yet didn't take that one
critical step to make it generally useful. :(

We could have a way to do this via the kernel, wherein the user has a
contract with us.  Basically we have a call to pass the Tags (what
granularity to use for this is a design point, pages, cache lines,
etc.)  into the kernel and the user agrees not to change them behind
the kernel's back.

In return the kernel agrees to restore the tags upon swapin.

So we could support something for swappable pages, it would just be
more work.

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

[Khalid Aziz]

On 03/07/2016 12:16 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:24:54 -0700


Tags can be cleared by user by setting tag to 0. Tags are
automatically cleared by the hardware when the mapping for a virtual
address is removed from TSB (which is why swappable pages are a
problem), so kernel does not have to do it as part of clean up.


You might be able to crib some bits for the Tag in the swp_entry_t, it's
64-bit and you can therefore steal bits from the offset field.

That way you'll have the ADI tag in the page tables, ready to re-install
at swapin time.



That is a possibility but limited in scope. An address range covered by 
a single TTE can have large number of tags. Version tags are set on 
cacheline. In extreme case, one could set a tag for each set of 64-bytes 
in a page. Also tags are set completely in userspace and no transition 
occurs to kernel space, so kernel has no idea of what tags have been 
set. I have not found a way to query the MMU on tags.


I will think some more about it.

Thanks,
Khalid

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:27:09 -0700

> I agree with your point of view. PSTATE.mcde and TTE.mcd are set in
> response to request from userspace. If userspace asked for them to be
> set, they already know but it was the database guys that asked for
> these two functions and they are the primary customers for the ADI
> feature. I am not crazy about this idea since this extends the
> mprotect API even further but would you consider using the return
> value from mprotect to indicate if PSTATE.mcde or TTE.mcd were already
> set on the given address?

Well, that's the idea.

If the mprotect using MAP_ADI or whatever succeeds, then ADI is
enabled.

Users can thus also pass MAP_ADI as a flag to mmap() to get ADI
protection from the very beginning.

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

[Khalid Aziz]

On 03/07/2016 12:16 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:24:54 -0700


Tags can be cleared by user by setting tag to 0. Tags are
automatically cleared by the hardware when the mapping for a virtual
address is removed from TSB (which is why swappable pages are a
problem), so kernel does not have to do it as part of clean up.


You might be able to crib some bits for the Tag in the swp_entry_t, it's
64-bit and you can therefore steal bits from the offset field.

That way you'll have the ADI tag in the page tables, ready to re-install
at swapin time.



That is a possibility but limited in scope. An address range covered by 
a single TTE can have large number of tags. Version tags are set on 
cacheline. In extreme case, one could set a tag for each set of 64-bytes 
in a page. Also tags are set completely in userspace and no transition 
occurs to kernel space, so kernel has no idea of what tags have been 
set. I have not found a way to query the MMU on tags.


I will think some more about it.

Thanks,
Khalid

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:27:09 -0700

> I agree with your point of view. PSTATE.mcde and TTE.mcd are set in
> response to request from userspace. If userspace asked for them to be
> set, they already know but it was the database guys that asked for
> these two functions and they are the primary customers for the ADI
> feature. I am not crazy about this idea since this extends the
> mprotect API even further but would you consider using the return
> value from mprotect to indicate if PSTATE.mcde or TTE.mcd were already
> set on the given address?

Well, that's the idea.

If the mprotect using MAP_ADI or whatever succeeds, then ADI is
enabled.

Users can thus also pass MAP_ADI as a flag to mmap() to get ADI
protection from the very beginning.

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

[Khalid Aziz]

On 03/07/2016 12:09 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:04:38 -0700


On 03/07/2016 09:56 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS


Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides
this in its return value in the patch I sent.


Unnecessary.  If any ADI mappings exist then mcde is set, otherwise it is
clear.  This is internal state and the application has no need to every
set nor query it.

It is implicit from the mprotect() calls the user makes to enable ADI
regions.


2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
provides this function in the patch I sent.


Again, implied by the mprotect() calls.



Hi Dave,

I agree with your point of view. PSTATE.mcde and TTE.mcd are set in 
response to request from userspace. If userspace asked for them to be 
set, they already know but it was the database guys that asked for these 
two functions and they are the primary customers for the ADI feature. I 
am not crazy about this idea since this extends the mprotect API even 
further but would you consider using the return value from mprotect to 
indicate if PSTATE.mcde or TTE.mcd were already set on the given address?


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 12:09 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:04:38 -0700


On 03/07/2016 09:56 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS


Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides
this in its return value in the patch I sent.


Unnecessary.  If any ADI mappings exist then mcde is set, otherwise it is
clear.  This is internal state and the application has no need to every
set nor query it.

It is implicit from the mprotect() calls the user makes to enable ADI
regions.


2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
provides this function in the patch I sent.


Again, implied by the mprotect() calls.



Hi Dave,

I agree with your point of view. PSTATE.mcde and TTE.mcd are set in 
response to request from userspace. If userspace asked for them to be 
set, they already know but it was the database guys that asked for these 
two functions and they are the primary customers for the ADI feature. I 
am not crazy about this idea since this extends the mprotect API even 
further but would you consider using the return value from mprotect to 
indicate if PSTATE.mcde or TTE.mcd were already set on the given address?


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 01:58 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 13:41:39 -0700


Shared data may not always be backed by a file. My understanding is
one of the use cases is for in-memory databases. This shared space
could also be used to hand off transactions in flight to other
processes. These transactions in flight would not be backed by a
file. Some of these use cases might not use shmfs even. Setting ADI
bits at virtual address level catches all these cases since what backs
the tagged virtual address can be anything - a mapped file, mmio
space, just plain chunk of memory.


Frankly the most interesting use case to me is simply finding bugs
and memory scribbles, and for that we're want to be able to ADI
arbitrary memory returned from malloc() and friends.

I personally see ADI more as a debugging than a security feature,
but that's just my view.



I think that is a very strong use case. It can be a very effective tool 
for debugging especially when it comes to catching wild writes.


--
Khalid

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

[Khalid Aziz]

On 03/07/2016 01:58 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 13:41:39 -0700


Shared data may not always be backed by a file. My understanding is
one of the use cases is for in-memory databases. This shared space
could also be used to hand off transactions in flight to other
processes. These transactions in flight would not be backed by a
file. Some of these use cases might not use shmfs even. Setting ADI
bits at virtual address level catches all these cases since what backs
the tagged virtual address can be anything - a mapped file, mmio
space, just plain chunk of memory.


Frankly the most interesting use case to me is simply finding bugs
and memory scribbles, and for that we're want to be able to ADI
arbitrary memory returned from malloc() and friends.

I personally see ADI more as a debugging than a security feature,
but that's just my view.



I think that is a very strong use case. It can be a very effective tool 
for debugging especially when it comes to catching wild writes.


--
Khalid

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

[Khalid Aziz]

On 03/07/2016 10:46 AM, Dave Hansen wrote:

On 03/07/2016 08:06 AM, Khalid Aziz wrote:

Top 4-bits of sparc64 virtual address are used for version tag only when
a process has its PSTATE.mcde bit set and it is accessing a memory
region that has ADI enabled on it (TTE.mcd set) and a version tag was
set on the virtual address being accessed. These 4-bits retain their
original semantics in all other cases.


OK, so this effectively reduces the address space of a process using the
feature.  Do we need to do anything explicit to keep an app from using
that address space?  Do we make sure the kernel doesn't place VMAs
there?  Do we respect mmap() hints that try to place memory there?



Good questions. Isn't set of valid VAs already constrained by VA_BITS 
(set to 44 in arch/sparc/include/asm/processor_64.h)? As I see it we are 
already not using the top 4 bits. Please correct me if I am wrong.


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 10:46 AM, Dave Hansen wrote:

On 03/07/2016 08:06 AM, Khalid Aziz wrote:

Top 4-bits of sparc64 virtual address are used for version tag only when
a process has its PSTATE.mcde bit set and it is accessing a memory
region that has ADI enabled on it (TTE.mcd set) and a version tag was
set on the virtual address being accessed. These 4-bits retain their
original semantics in all other cases.


OK, so this effectively reduces the address space of a process using the
feature.  Do we need to do anything explicit to keep an app from using
that address space?  Do we make sure the kernel doesn't place VMAs
there?  Do we respect mmap() hints that try to place memory there?



Good questions. Isn't set of valid VAs already constrained by VA_BITS 
(set to 44 in arch/sparc/include/asm/processor_64.h)? As I see it we are 
already not using the top 4 bits. Please correct me if I am wrong.


Thanks,
Khalid

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

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 12:58 PM, David Miller  wrote:
> From: Khalid Aziz 
> Date: Mon, 7 Mar 2016 13:41:39 -0700
>
>> Shared data may not always be backed by a file. My understanding is
>> one of the use cases is for in-memory databases. This shared space
>> could also be used to hand off transactions in flight to other
>> processes. These transactions in flight would not be backed by a
>> file. Some of these use cases might not use shmfs even. Setting ADI
>> bits at virtual address level catches all these cases since what backs
>> the tagged virtual address can be anything - a mapped file, mmio
>> space, just plain chunk of memory.
>
> Frankly the most interesting use case to me is simply finding bugs
> and memory scribbles, and for that we're want to be able to ADI
> arbitrary memory returned from malloc() and friends.
>
> I personally see ADI more as a debugging than a security feature,
> but that's just my view.

The thing that seems awkward to me is that setting, say, ADI=1 seems
almost equivalent to remapping the memory up to 0x10...whatever, and
the latter is a heck of a lot simpler to think about.

-- 
Andy Lutomirski
AMA Capital Management, LLC

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

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 12:58 PM, David Miller  wrote:
> From: Khalid Aziz 
> Date: Mon, 7 Mar 2016 13:41:39 -0700
>
>> Shared data may not always be backed by a file. My understanding is
>> one of the use cases is for in-memory databases. This shared space
>> could also be used to hand off transactions in flight to other
>> processes. These transactions in flight would not be backed by a
>> file. Some of these use cases might not use shmfs even. Setting ADI
>> bits at virtual address level catches all these cases since what backs
>> the tagged virtual address can be anything - a mapped file, mmio
>> space, just plain chunk of memory.
>
> Frankly the most interesting use case to me is simply finding bugs
> and memory scribbles, and for that we're want to be able to ADI
> arbitrary memory returned from malloc() and friends.
>
> I personally see ADI more as a debugging than a security feature,
> but that's just my view.

The thing that seems awkward to me is that setting, say, ADI=1 seems
almost equivalent to remapping the memory up to 0x10...whatever, and
the latter is a heck of a lot simpler to think about.

-- 
Andy Lutomirski
AMA Capital Management, LLC

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 13:41:39 -0700

> Shared data may not always be backed by a file. My understanding is
> one of the use cases is for in-memory databases. This shared space
> could also be used to hand off transactions in flight to other
> processes. These transactions in flight would not be backed by a
> file. Some of these use cases might not use shmfs even. Setting ADI
> bits at virtual address level catches all these cases since what backs
> the tagged virtual address can be anything - a mapped file, mmio
> space, just plain chunk of memory.

Frankly the most interesting use case to me is simply finding bugs
and memory scribbles, and for that we're want to be able to ADI
arbitrary memory returned from malloc() and friends.

I personally see ADI more as a debugging than a security feature,
but that's just my view.

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 13:41:39 -0700

> Shared data may not always be backed by a file. My understanding is
> one of the use cases is for in-memory databases. This shared space
> could also be used to hand off transactions in flight to other
> processes. These transactions in flight would not be backed by a
> file. Some of these use cases might not use shmfs even. Setting ADI
> bits at virtual address level catches all these cases since what backs
> the tagged virtual address can be anything - a mapped file, mmio
> space, just plain chunk of memory.

Frankly the most interesting use case to me is simply finding bugs
and memory scribbles, and for that we're want to be able to ADI
arbitrary memory returned from malloc() and friends.

I personally see ADI more as a debugging than a security feature,
but that's just my view.

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

[Khalid Aziz]

On 03/07/2016 12:54 PM, Andy Lutomirski wrote:

On Mon, Mar 7, 2016 at 11:44 AM, Khalid Aziz  wrote:


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?


This makes sense, but I still think the design is poor.  If the hacker
gets code execution, then they can trivially brute force the ADI bits.


True, with only 16 possible tag values (actually only 14 since 0 and 15 
are reserved values), it is entirely possible to brute force the ADI 
tag. ADI is just another tool one can use to mitigate attacks. A process 
that accesses an ADI enabled memory with invalid tag gets a SIGBUS and 
is terminated. This can trigger alerts on the system and system policies 
could block the next attack. If a daemon is compromised and is forced to 
hand out data from memory it should not be reading (similar to 
heartbleed bug). the daemon itself is terminated with SIGBUS which 
should be enough to alert system admins. A rotating set of tags would 
reduce the risk from brute force attacks. Tags are set on cacheline 
(which is 64 bytes on M7). A single regular sized page can have 128 sets 
of tags. Allowing for 14 possible values for each set, that is a lot of 
possible combinations of tags making it very hard to brute force tags 
for more than a cacheline at a time. There are probably other better 
ways to make the tags harder to crack.




Also, if this is the use case in mind, shouldn't the ADI bits bet set
on the file, not the mapping?  E.g. have an ioctl on the shmfs file
that sets its ADI bits?


Shared data may not always be backed by a file. My understanding is one 
of the use cases is for in-memory databases. This shared space could 
also be used to hand off transactions in flight to other processes. 
These transactions in flight would not be backed by a file. Some of 
these use cases might not use shmfs even. Setting ADI bits at virtual 
address level catches all these cases since what backs the tagged 
virtual address can be anything - a mapped file, mmio space, just plain 
chunk of memory.





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?


OK, that makes some sense.

Can a shared page ever have TTE.mcd set?  How does one share a page,
even deliberately, between two processes with cmd set?


For two processes to share a page, their VMAs have to be identical as I 
understand it. If one process has TTE.mcd set (which means vma->vm_flags 
is different) while the other does not, they do not share a page.


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 12:54 PM, Andy Lutomirski wrote:

On Mon, Mar 7, 2016 at 11:44 AM, Khalid Aziz  wrote:


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?


This makes sense, but I still think the design is poor.  If the hacker
gets code execution, then they can trivially brute force the ADI bits.


True, with only 16 possible tag values (actually only 14 since 0 and 15 
are reserved values), it is entirely possible to brute force the ADI 
tag. ADI is just another tool one can use to mitigate attacks. A process 
that accesses an ADI enabled memory with invalid tag gets a SIGBUS and 
is terminated. This can trigger alerts on the system and system policies 
could block the next attack. If a daemon is compromised and is forced to 
hand out data from memory it should not be reading (similar to 
heartbleed bug). the daemon itself is terminated with SIGBUS which 
should be enough to alert system admins. A rotating set of tags would 
reduce the risk from brute force attacks. Tags are set on cacheline 
(which is 64 bytes on M7). A single regular sized page can have 128 sets 
of tags. Allowing for 14 possible values for each set, that is a lot of 
possible combinations of tags making it very hard to brute force tags 
for more than a cacheline at a time. There are probably other better 
ways to make the tags harder to crack.




Also, if this is the use case in mind, shouldn't the ADI bits bet set
on the file, not the mapping?  E.g. have an ioctl on the shmfs file
that sets its ADI bits?


Shared data may not always be backed by a file. My understanding is one 
of the use cases is for in-memory databases. This shared space could 
also be used to hand off transactions in flight to other processes. 
These transactions in flight would not be backed by a file. Some of 
these use cases might not use shmfs even. Setting ADI bits at virtual 
address level catches all these cases since what backs the tagged 
virtual address can be anything - a mapped file, mmio space, just plain 
chunk of memory.





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?


OK, that makes some sense.

Can a shared page ever have TTE.mcd set?  How does one share a page,
even deliberately, between two processes with cmd set?


For two processes to share a page, their VMAs have to be identical as I 
understand it. If one process has TTE.mcd set (which means vma->vm_flags 
is different) while the other does not, they do not share a page.


Thanks,
Khalid

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

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 11:44 AM, Khalid Aziz  wrote:
> On 03/07/2016 11:49 AM, Andy Lutomirski wrote:
>>
>> On Mon, Mar 7, 2016 at 10:22 AM, Khalid Aziz 
>> 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?

This makes sense, but I still think the design is poor.  If the hacker
gets code execution, then they can trivially brute force the ADI bits.

Also, if this is the use case in mind, shouldn't the ADI bits bet set
on the file, not the mapping?  E.g. have an ioctl on the shmfs file
that sets its ADI bits?

>
>>>

 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?

OK, that makes some sense.

Can a shared page ever have TTE.mcd set?  How does one share a page,
even deliberately, between two processes with cmd set?

--Andy

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

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 11:44 AM, Khalid Aziz  wrote:
> On 03/07/2016 11:49 AM, Andy Lutomirski wrote:
>>
>> On Mon, Mar 7, 2016 at 10:22 AM, Khalid Aziz 
>> 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?

This makes sense, but I still think the design is poor.  If the hacker
gets code execution, then they can trivially brute force the ADI bits.

Also, if this is the use case in mind, shouldn't the ADI bits bet set
on the file, not the mapping?  E.g. have an ioctl on the shmfs file
that sets its ADI bits?

>
>>>

 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?

OK, that makes some sense.

Can a shared page ever have TTE.mcd set?  How does one share a page,
even deliberately, between two processes with cmd set?

--Andy

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

[Khalid Aziz]

On 03/07/2016 12:22 PM, David Miller wrote:

Khalid, maybe you should share notes with the folks working on x86
protection keys.



Good idea. Sparc ADI feature is indeed similar to x86 protection keys 
sounds like.


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 12:22 PM, David Miller wrote:

Khalid, maybe you should share notes with the folks working on x86
protection keys.



Good idea. Sparc ADI feature is indeed similar to x86 protection keys 
sounds like.


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 11:49 AM, Andy Lutomirski wrote:

On Mon, Mar 7, 2016 at 10:22 AM, Khalid Aziz  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

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

[Khalid Aziz]

On 03/07/2016 11:49 AM, Andy Lutomirski wrote:

On Mon, Mar 7, 2016 at 10:22 AM, Khalid Aziz  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

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

[David Miller]

From: Andy Lutomirski 
Date: Mon, 7 Mar 2016 10:53:23 -0800

> x86 has an upcoming feature called protection keys.  A page of virtual
> memory has a protection key, which is a number from 0 through 16.  The
> master copy is in the PTE, i.e. page table entry, which is a
> software-managed data structure in memory and is exactly the thing
> that Linux calls "pte".  The processor can cache that value in the TLB
> (translation lookaside buffer), which is a hardware cache that caches
> PTEs.  On access to a page of virtual memory, the processor does a
> certain calculation involving a new register called PKRU and the
> protection key and may deny access.

ADI is similar, except the "keys" (or "tags") are stored externally
rather than in the PTEs.  A bit in the PTE is used to enable tag match
checking.

The tags live in an external table, which is populated by ASI store
instructions.  The location of the table is implementation specific,
it could be hypervisor or CPU managed, but if stored in memory it is
to a region of memory accessible only to the hypervisor at best.

Khalid, maybe you should share notes with the folks working on x86
protection keys.

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

[David Miller]

From: Andy Lutomirski 
Date: Mon, 7 Mar 2016 10:53:23 -0800

> x86 has an upcoming feature called protection keys.  A page of virtual
> memory has a protection key, which is a number from 0 through 16.  The
> master copy is in the PTE, i.e. page table entry, which is a
> software-managed data structure in memory and is exactly the thing
> that Linux calls "pte".  The processor can cache that value in the TLB
> (translation lookaside buffer), which is a hardware cache that caches
> PTEs.  On access to a page of virtual memory, the processor does a
> certain calculation involving a new register called PKRU and the
> protection key and may deny access.

ADI is similar, except the "keys" (or "tags") are stored externally
rather than in the PTEs.  A bit in the PTE is used to enable tag match
checking.

The tags live in an external table, which is populated by ASI store
instructions.  The location of the table is implementation specific,
it could be hypervisor or CPU managed, but if stored in memory it is
to a region of memory accessible only to the hypervisor at best.

Khalid, maybe you should share notes with the folks working on x86
protection keys.

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

[David Miller]

From: Andy Lutomirski 
Date: Mon, 7 Mar 2016 10:49:57 -0800

> What data structure or structures changes when this stxa instruction happens?

An internal table, maintained by the CPU and/or hypervisor, and if in physical
addresses then in a region which is only accessible by the hypervisor.

The table is not accessible by the kernel at all via loads or stores.

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

[David Miller]

From: Andy Lutomirski 
Date: Mon, 7 Mar 2016 10:49:57 -0800

> What data structure or structures changes when this stxa instruction happens?

An internal table, maintained by the CPU and/or hypervisor, and if in physical
addresses then in a region which is only accessible by the hypervisor.

The table is not accessible by the kernel at all via loads or stores.

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:24:54 -0700

> Tags can be cleared by user by setting tag to 0. Tags are
> automatically cleared by the hardware when the mapping for a virtual
> address is removed from TSB (which is why swappable pages are a
> problem), so kernel does not have to do it as part of clean up.

You might be able to crib some bits for the Tag in the swp_entry_t, it's
64-bit and you can therefore steal bits from the offset field.

That way you'll have the ADI tag in the page tables, ready to re-install
at swapin time.

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:24:54 -0700

> Tags can be cleared by user by setting tag to 0. Tags are
> automatically cleared by the hardware when the mapping for a virtual
> address is removed from TSB (which is why swappable pages are a
> problem), so kernel does not have to do it as part of clean up.

You might be able to crib some bits for the Tag in the swp_entry_t, it's
64-bit and you can therefore steal bits from the offset field.

That way you'll have the ADI tag in the page tables, ready to re-install
at swapin time.

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:04:38 -0700

> On 03/07/2016 09:56 AM, David Miller wrote:
>> From: Khalid Aziz 
>> Date: Mon, 7 Mar 2016 08:07:53 -0700
>>
>>> PR_GET_SPARC_ADICAPS
>>
>> Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.
>>
>> So now all that's left is supposedly the TAG stuff, please explain
>> that to me so I can direct you to the correct existing interface to
>> provide that as well.
>>
>> Really, try to avoid prtctl, it's poorly typed and almost worse than
>> ioctl().
>>
> 
> The two remaining operations I am looking at are:
> 
> 1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides
> this in its return value in the patch I sent.

Unnecessary.  If any ADI mappings exist then mcde is set, otherwise it is
clear.  This is internal state and the application has no need to every
set nor query it.

It is implicit from the mprotect() calls the user makes to enable ADI
regions.

> 2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
> provides this function in the patch I sent.

Again, implied by the mprotect() calls.

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:04:38 -0700

> On 03/07/2016 09:56 AM, David Miller wrote:
>> From: Khalid Aziz 
>> Date: Mon, 7 Mar 2016 08:07:53 -0700
>>
>>> PR_GET_SPARC_ADICAPS
>>
>> Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.
>>
>> So now all that's left is supposedly the TAG stuff, please explain
>> that to me so I can direct you to the correct existing interface to
>> provide that as well.
>>
>> Really, try to avoid prtctl, it's poorly typed and almost worse than
>> ioctl().
>>
> 
> The two remaining operations I am looking at are:
> 
> 1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides
> this in its return value in the patch I sent.

Unnecessary.  If any ADI mappings exist then mcde is set, otherwise it is
clear.  This is internal state and the application has no need to every
set nor query it.

It is implicit from the mprotect() calls the user makes to enable ADI
regions.

> 2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
> provides this function in the patch I sent.

Again, implied by the mprotect() calls.

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

[David Miller]

From: Dave Hansen 
Date: Mon, 7 Mar 2016 09:35:57 -0800

> On 03/02/2016 12:39 PM, Khalid Aziz wrote:
>> +long enable_sparc_adi(unsigned long addr, unsigned long len)
>> +{
>> +unsigned long end, pagemask;
>> +int error;
>> +struct vm_area_struct *vma, *vma2;
>> +struct mm_struct *mm;
>> +
>> +if (!ADI_CAPABLE())
>> +return -EINVAL;
> ...
> 
> This whole thing with the VMA splitting and so forth looks pretty darn
> arch-independent.  Are you sure you need that much arch-specific code
> for it, or can you share more of the generic VMA management code?

This is exactly what I have suggested to him, and he has agreed to pursue.

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

[David Miller]

From: Dave Hansen 
Date: Mon, 7 Mar 2016 09:35:57 -0800

> On 03/02/2016 12:39 PM, Khalid Aziz wrote:
>> +long enable_sparc_adi(unsigned long addr, unsigned long len)
>> +{
>> +unsigned long end, pagemask;
>> +int error;
>> +struct vm_area_struct *vma, *vma2;
>> +struct mm_struct *mm;
>> +
>> +if (!ADI_CAPABLE())
>> +return -EINVAL;
> ...
> 
> This whole thing with the VMA splitting and so forth looks pretty darn
> arch-independent.  Are you sure you need that much arch-specific code
> for it, or can you share more of the generic VMA management code?

This is exactly what I have suggested to him, and he has agreed to pursue.

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

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 10:39 AM, Khalid Aziz  wrote:
> On 03/07/2016 11:12 AM, Dave Hansen wrote:
>>
>> On 03/07/2016 09:53 AM, Andy Lutomirski wrote:
>>>
>>> Also, what am I missing?  Tying these tags to the physical page seems
>>> like a poor design to me.  This seems really awkward to use.
>>
>>
>> Yeah, can you describe the structures that store these things?  Surely
>> the hardware has some kind of lookup tables for them and stores them in
>> memory _somewhere_.
>>
>
> Version tags are tied to virtual addresses, not physical pages.
>
> Where exactly are the tags stored is part of processor architecture and I am
> not privy to that. MMU stores these lookup tables somewhere and uses it to
> authenticate access to virtual addresses. It really is irrelevant to kernel
> how MMU implements access controls as long as we have access to the
> knowledge of how to use it.
>

Can you translate this for people who don't know all the SPARC acronyms?

x86 has an upcoming feature called protection keys.  A page of virtual
memory has a protection key, which is a number from 0 through 16.  The
master copy is in the PTE, i.e. page table entry, which is a
software-managed data structure in memory and is exactly the thing
that Linux calls "pte".  The processor can cache that value in the TLB
(translation lookaside buffer), which is a hardware cache that caches
PTEs.  On access to a page of virtual memory, the processor does a
certain calculation involving a new register called PKRU and the
protection key and may deny access.

Hopefully that description makes sense even to people completely
unfamiliar with x86.

Can you try something similar for SPARC?  So far I'm lost, because
you've said that the ADI tag is associated with a VA, but it has to
match for aliases, and you've mentioned a bunch of acronyms, and I
have no clue what's going on.

--Andy

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

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 10:39 AM, Khalid Aziz  wrote:
> On 03/07/2016 11:12 AM, Dave Hansen wrote:
>>
>> On 03/07/2016 09:53 AM, Andy Lutomirski wrote:
>>>
>>> Also, what am I missing?  Tying these tags to the physical page seems
>>> like a poor design to me.  This seems really awkward to use.
>>
>>
>> Yeah, can you describe the structures that store these things?  Surely
>> the hardware has some kind of lookup tables for them and stores them in
>> memory _somewhere_.
>>
>
> Version tags are tied to virtual addresses, not physical pages.
>
> Where exactly are the tags stored is part of processor architecture and I am
> not privy to that. MMU stores these lookup tables somewhere and uses it to
> authenticate access to virtual addresses. It really is irrelevant to kernel
> how MMU implements access controls as long as we have access to the
> knowledge of how to use it.
>

Can you translate this for people who don't know all the SPARC acronyms?

x86 has an upcoming feature called protection keys.  A page of virtual
memory has a protection key, which is a number from 0 through 16.  The
master copy is in the PTE, i.e. page table entry, which is a
software-managed data structure in memory and is exactly the thing
that Linux calls "pte".  The processor can cache that value in the TLB
(translation lookaside buffer), which is a hardware cache that caches
PTEs.  On access to a page of virtual memory, the processor does a
certain calculation involving a new register called PKRU and the
protection key and may deny access.

Hopefully that description makes sense even to people completely
unfamiliar with x86.

Can you try something similar for SPARC?  So far I'm lost, because
you've said that the ADI tag is associated with a VA, but it has to
match for aliases, and you've mentioned a bunch of acronyms, and I
have no clue what's going on.

--Andy

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

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 10:22 AM, Khalid Aziz  wrote:
> On 03/07/2016 11:08 AM, Andy Lutomirski wrote:
>>
>> On Mon, Mar 7, 2016 at 10:04 AM, Khalid Aziz 
>> wrote:
>>>
>>> On 03/07/2016 09:56 AM, David Miller wrote:


 From: Khalid Aziz 
 Date: Mon, 7 Mar 2016 08:07:53 -0700

> PR_GET_SPARC_ADICAPS



 Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

 So now all that's left is supposedly the TAG stuff, please explain
 that to me so I can direct you to the correct existing interface to
 provide that as well.

 Really, try to avoid prtctl, it's poorly typed and almost worse than
 ioctl().

>>>
>>> The two remaining operations I am looking at are:
>>>
>>> 1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides this
>>> in
>>> its return value in the patch I sent.
>>>
>>> 2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
>>> provides this function in the patch I sent.
>>>
>>> Setting and clearing version tags can be done entirely from userspace:
>>>
>>>  while (addr < end) {
>>>  asm volatile(
>>>  "stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
>>>  :
>>>  : "r" (addr), "r" (version));
>>>  addr += adicap.blksz;
>>>  }
>>> so I do not have to add any kernel code for tags.
>>
>>
>> Is the effect of that to change the tag associated with a page to
>> which the caller has write access?
>
>
> 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?

>
>>
>> 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.

What data structure or structures changes when this stxa instruction happens?

--Andy

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

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 10:22 AM, Khalid Aziz  wrote:
> On 03/07/2016 11:08 AM, Andy Lutomirski wrote:
>>
>> On Mon, Mar 7, 2016 at 10:04 AM, Khalid Aziz 
>> wrote:
>>>
>>> On 03/07/2016 09:56 AM, David Miller wrote:


 From: Khalid Aziz 
 Date: Mon, 7 Mar 2016 08:07:53 -0700

> PR_GET_SPARC_ADICAPS



 Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

 So now all that's left is supposedly the TAG stuff, please explain
 that to me so I can direct you to the correct existing interface to
 provide that as well.

 Really, try to avoid prtctl, it's poorly typed and almost worse than
 ioctl().

>>>
>>> The two remaining operations I am looking at are:
>>>
>>> 1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides this
>>> in
>>> its return value in the patch I sent.
>>>
>>> 2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
>>> provides this function in the patch I sent.
>>>
>>> Setting and clearing version tags can be done entirely from userspace:
>>>
>>>  while (addr < end) {
>>>  asm volatile(
>>>  "stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
>>>  :
>>>  : "r" (addr), "r" (version));
>>>  addr += adicap.blksz;
>>>  }
>>> so I do not have to add any kernel code for tags.
>>
>>
>> Is the effect of that to change the tag associated with a page to
>> which the caller has write access?
>
>
> 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?

>
>>
>> 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.

What data structure or structures changes when this stxa instruction happens?

--Andy

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

[Khalid Aziz]

On 03/07/2016 11:12 AM, Dave Hansen wrote:

On 03/07/2016 09:53 AM, Andy Lutomirski wrote:

Also, what am I missing?  Tying these tags to the physical page seems
like a poor design to me.  This seems really awkward to use.


Yeah, can you describe the structures that store these things?  Surely
the hardware has some kind of lookup tables for them and stores them in
memory _somewhere_.



Version tags are tied to virtual addresses, not physical pages.

Where exactly are the tags stored is part of processor architecture and 
I am not privy to that. MMU stores these lookup tables somewhere and 
uses it to authenticate access to virtual addresses. It really is 
irrelevant to kernel how MMU implements access controls as long as we 
have access to the knowledge of how to use it.


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 11:12 AM, Dave Hansen wrote:

On 03/07/2016 09:53 AM, Andy Lutomirski wrote:

Also, what am I missing?  Tying these tags to the physical page seems
like a poor design to me.  This seems really awkward to use.


Yeah, can you describe the structures that store these things?  Surely
the hardware has some kind of lookup tables for them and stores them in
memory _somewhere_.



Version tags are tied to virtual addresses, not physical pages.

Where exactly are the tags stored is part of processor architecture and 
I am not privy to that. MMU stores these lookup tables somewhere and 
uses it to authenticate access to virtual addresses. It really is 
irrelevant to kernel how MMU implements access controls as long as we 
have access to the knowledge of how to use it.


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 11:09 AM, Rob Gardner wrote:

On 03/07/2016 10:04 AM, Khalid Aziz wrote:

On 03/07/2016 09:56 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS


Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides
this in its return value in the patch I sent.

2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
provides this function in the patch I sent.

Setting and clearing version tags can be done entirely from userspace:

while (addr < end) {
asm volatile(
"stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
:
: "r" (addr), "r" (version));
addr += adicap.blksz;
}
so I do not have to add any kernel code for tags.



What about clearing the tags when the user is done with the memory? You
can't count on the user to do that, so doesn't the kernel have to do it
someplace?



Tags can be cleared by user by setting tag to 0. Tags are automatically 
cleared by the hardware when the mapping for a virtual address is 
removed from TSB (which is why swappable pages are a problem), so kernel 
does not have to do it as part of clean up.


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 11:09 AM, Rob Gardner wrote:

On 03/07/2016 10:04 AM, Khalid Aziz wrote:

On 03/07/2016 09:56 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS


Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides
this in its return value in the patch I sent.

2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
provides this function in the patch I sent.

Setting and clearing version tags can be done entirely from userspace:

while (addr < end) {
asm volatile(
"stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
:
: "r" (addr), "r" (version));
addr += adicap.blksz;
}
so I do not have to add any kernel code for tags.



What about clearing the tags when the user is done with the memory? You
can't count on the user to do that, so doesn't the kernel have to do it
someplace?



Tags can be cleared by user by setting tag to 0. Tags are automatically 
cleared by the hardware when the mapping for a virtual address is 
removed from TSB (which is why swappable pages are a problem), so kernel 
does not have to do it as part of clean up.


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 11:08 AM, Andy Lutomirski wrote:

On Mon, Mar 7, 2016 at 10:04 AM, Khalid Aziz  wrote:

On 03/07/2016 09:56 AM, David Miller wrote:


From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS



Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides this in
its return value in the patch I sent.

2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
provides this function in the patch I sent.

Setting and clearing version tags can be done entirely from userspace:

 while (addr < end) {
 asm volatile(
 "stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
 :
 : "r" (addr), "r" (version));
 addr += adicap.blksz;
 }
so I do not have to add any kernel code for tags.


Is the effect of that to change the tag associated with a page to
which the caller has write access?


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.




I sense DoS issues in your future.



Are you concerned about DoS even if the tag is associated with virtual 
address, not physical address?


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 11:08 AM, Andy Lutomirski wrote:

On Mon, Mar 7, 2016 at 10:04 AM, Khalid Aziz  wrote:

On 03/07/2016 09:56 AM, David Miller wrote:


From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS



Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides this in
its return value in the patch I sent.

2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
provides this function in the patch I sent.

Setting and clearing version tags can be done entirely from userspace:

 while (addr < end) {
 asm volatile(
 "stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
 :
 : "r" (addr), "r" (version));
 addr += adicap.blksz;
 }
so I do not have to add any kernel code for tags.


Is the effect of that to change the tag associated with a page to
which the caller has write access?


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.




I sense DoS issues in your future.



Are you concerned about DoS even if the tag is associated with virtual 
address, not physical address?


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 10:35 AM, Dave Hansen wrote:

On 03/02/2016 12:39 PM, Khalid Aziz wrote:

+long enable_sparc_adi(unsigned long addr, unsigned long len)
+{
+   unsigned long end, pagemask;
+   int error;
+   struct vm_area_struct *vma, *vma2;
+   struct mm_struct *mm;
+
+   if (!ADI_CAPABLE())
+   return -EINVAL;

...

This whole thing with the VMA splitting and so forth looks pretty darn
arch-independent.  Are you sure you need that much arch-specific code
for it, or can you share more of the generic VMA management code?



All of the VMA splitting/merging code is rather generic and is very 
similar to the code for mbind, mlock, madavise and mprotect. Currently 
there is no code sharing across all of these implementations. Maybe that 
should change. In any case, I am looking at changing the interface to go 
through mprotect instead as Dave suggested. I can share the code in 
mprotect in that case. The only arch dependent part will be to set the 
VM_SPARC_ADI flag on the VMA.


Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 10:35 AM, Dave Hansen wrote:

On 03/02/2016 12:39 PM, Khalid Aziz wrote:

+long enable_sparc_adi(unsigned long addr, unsigned long len)
+{
+   unsigned long end, pagemask;
+   int error;
+   struct vm_area_struct *vma, *vma2;
+   struct mm_struct *mm;
+
+   if (!ADI_CAPABLE())
+   return -EINVAL;

...

This whole thing with the VMA splitting and so forth looks pretty darn
arch-independent.  Are you sure you need that much arch-specific code
for it, or can you share more of the generic VMA management code?



All of the VMA splitting/merging code is rather generic and is very 
similar to the code for mbind, mlock, madavise and mprotect. Currently 
there is no code sharing across all of these implementations. Maybe that 
should change. In any case, I am looking at changing the interface to go 
through mprotect instead as Dave suggested. I can share the code in 
mprotect in that case. The only arch dependent part will be to set the 
VM_SPARC_ADI flag on the VMA.


Thanks,
Khalid

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

[Rob Gardner]

On 03/07/2016 10:04 AM, Khalid Aziz wrote:

On 03/07/2016 09:56 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS


Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides 
this in its return value in the patch I sent.


2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS 
provides this function in the patch I sent.


Setting and clearing version tags can be done entirely from userspace:

while (addr < end) {
asm volatile(
"stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
:
: "r" (addr), "r" (version));
addr += adicap.blksz;
}
so I do not have to add any kernel code for tags.



What about clearing the tags when the user is done with the memory? You 
can't count on the user to do that, so doesn't the kernel have to do it 
someplace?


Rob

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

[Rob Gardner]

On 03/07/2016 10:04 AM, Khalid Aziz wrote:

On 03/07/2016 09:56 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS


Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides 
this in its return value in the patch I sent.


2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS 
provides this function in the patch I sent.


Setting and clearing version tags can be done entirely from userspace:

while (addr < end) {
asm volatile(
"stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
:
: "r" (addr), "r" (version));
addr += adicap.blksz;
}
so I do not have to add any kernel code for tags.



What about clearing the tags when the user is done with the memory? You 
can't count on the user to do that, so doesn't the kernel have to do it 
someplace?


Rob

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

[Dave Hansen]

On 03/07/2016 09:53 AM, Andy Lutomirski wrote:
> Also, what am I missing?  Tying these tags to the physical page seems
> like a poor design to me.  This seems really awkward to use.

Yeah, can you describe the structures that store these things?  Surely
the hardware has some kind of lookup tables for them and stores them in
memory _somewhere_.

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

[Dave Hansen]

On 03/07/2016 09:53 AM, Andy Lutomirski wrote:
> Also, what am I missing?  Tying these tags to the physical page seems
> like a poor design to me.  This seems really awkward to use.

Yeah, can you describe the structures that store these things?  Surely
the hardware has some kind of lookup tables for them and stores them in
memory _somewhere_.

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

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 10:04 AM, Khalid Aziz  wrote:
> On 03/07/2016 09:56 AM, David Miller wrote:
>>
>> From: Khalid Aziz 
>> Date: Mon, 7 Mar 2016 08:07:53 -0700
>>
>>> PR_GET_SPARC_ADICAPS
>>
>>
>> Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.
>>
>> So now all that's left is supposedly the TAG stuff, please explain
>> that to me so I can direct you to the correct existing interface to
>> provide that as well.
>>
>> Really, try to avoid prtctl, it's poorly typed and almost worse than
>> ioctl().
>>
>
> The two remaining operations I am looking at are:
>
> 1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides this in
> its return value in the patch I sent.
>
> 2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
> provides this function in the patch I sent.
>
> Setting and clearing version tags can be done entirely from userspace:
>
> while (addr < end) {
> asm volatile(
> "stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
> :
> : "r" (addr), "r" (version));
> addr += adicap.blksz;
> }
> so I do not have to add any kernel code for tags.

Is the effect of that to change the tag associated with a page to
which the caller has write access?

I sense DoS issues in your future.

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

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 10:04 AM, Khalid Aziz  wrote:
> On 03/07/2016 09:56 AM, David Miller wrote:
>>
>> From: Khalid Aziz 
>> Date: Mon, 7 Mar 2016 08:07:53 -0700
>>
>>> PR_GET_SPARC_ADICAPS
>>
>>
>> Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.
>>
>> So now all that's left is supposedly the TAG stuff, please explain
>> that to me so I can direct you to the correct existing interface to
>> provide that as well.
>>
>> Really, try to avoid prtctl, it's poorly typed and almost worse than
>> ioctl().
>>
>
> The two remaining operations I am looking at are:
>
> 1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides this in
> its return value in the patch I sent.
>
> 2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
> provides this function in the patch I sent.
>
> Setting and clearing version tags can be done entirely from userspace:
>
> while (addr < end) {
> asm volatile(
> "stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
> :
> : "r" (addr), "r" (version));
> addr += adicap.blksz;
> }
> so I do not have to add any kernel code for tags.

Is the effect of that to change the tag associated with a page to
which the caller has write access?

I sense DoS issues in your future.

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

[Khalid Aziz]

On 03/07/2016 09:56 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS


Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides 
this in its return value in the patch I sent.


2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS 
provides this function in the patch I sent.


Setting and clearing version tags can be done entirely from userspace:

while (addr < end) {
asm volatile(
"stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
:
: "r" (addr), "r" (version));
addr += adicap.blksz;
}
so I do not have to add any kernel code for tags.

Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 09:56 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS


Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides 
this in its return value in the patch I sent.


2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS 
provides this function in the patch I sent.


Setting and clearing version tags can be done entirely from userspace:

while (addr < end) {
asm volatile(
"stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
:
: "r" (addr), "r" (version));
addr += adicap.blksz;
}
so I do not have to add any kernel code for tags.

Thanks,
Khalid

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

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 9:46 AM, Dave Hansen  wrote:
> On 03/07/2016 08:06 AM, Khalid Aziz wrote:
>> Top 4-bits of sparc64 virtual address are used for version tag only when
>> a process has its PSTATE.mcde bit set and it is accessing a memory
>> region that has ADI enabled on it (TTE.mcd set) and a version tag was
>> set on the virtual address being accessed. These 4-bits retain their
>> original semantics in all other cases.
>
> OK, so this effectively reduces the address space of a process using the
> feature.  Do we need to do anything explicit to keep an app from using
> that address space?  Do we make sure the kernel doesn't place VMAs
> there?  Do we respect mmap() hints that try to place memory there?

Also, what happens when someone does this to an aliased page?  This
could be a MAP_SHARED mapping or a not-yet-COWed MAP_ANONYMOUS
mapping.

Also, what am I missing?  Tying these tags to the physical page seems
like a poor design to me.  This seems really awkward to use.

-- 
Andy Lutomirski
AMA Capital Management, LLC

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

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 9:46 AM, Dave Hansen  wrote:
> On 03/07/2016 08:06 AM, Khalid Aziz wrote:
>> Top 4-bits of sparc64 virtual address are used for version tag only when
>> a process has its PSTATE.mcde bit set and it is accessing a memory
>> region that has ADI enabled on it (TTE.mcd set) and a version tag was
>> set on the virtual address being accessed. These 4-bits retain their
>> original semantics in all other cases.
>
> OK, so this effectively reduces the address space of a process using the
> feature.  Do we need to do anything explicit to keep an app from using
> that address space?  Do we make sure the kernel doesn't place VMAs
> there?  Do we respect mmap() hints that try to place memory there?

Also, what happens when someone does this to an aliased page?  This
could be a MAP_SHARED mapping or a not-yet-COWed MAP_ANONYMOUS
mapping.

Also, what am I missing?  Tying these tags to the physical page seems
like a poor design to me.  This seems really awkward to use.

-- 
Andy Lutomirski
AMA Capital Management, LLC

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

[Khalid Aziz]

On 03/07/2016 09:45 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


I can remove CONFIG_SPARC_ADI. It does mean this code will be built
into 32-bit kernels as well but it will be inactive code.


The code should be built only into obj-$(CONFIG_SPARC64) just like the
rest of the 64-bit specific code.  I don't know why in the world you
would build it into the 32-bit kernel.



You are right. I did not understand you correctly the first time.

Thanks,
Khalid

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

[Khalid Aziz]

On 03/07/2016 09:45 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


I can remove CONFIG_SPARC_ADI. It does mean this code will be built
into 32-bit kernels as well but it will be inactive code.


The code should be built only into obj-$(CONFIG_SPARC64) just like the
rest of the 64-bit specific code.  I don't know why in the world you
would build it into the 32-bit kernel.



You are right. I did not understand you correctly the first time.

Thanks,
Khalid

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

[Dave Hansen]

On 03/07/2016 08:06 AM, Khalid Aziz wrote:
> Top 4-bits of sparc64 virtual address are used for version tag only when
> a process has its PSTATE.mcde bit set and it is accessing a memory
> region that has ADI enabled on it (TTE.mcd set) and a version tag was
> set on the virtual address being accessed. These 4-bits retain their
> original semantics in all other cases.

OK, so this effectively reduces the address space of a process using the
feature.  Do we need to do anything explicit to keep an app from using
that address space?  Do we make sure the kernel doesn't place VMAs
there?  Do we respect mmap() hints that try to place memory there?

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

[Dave Hansen]

On 03/07/2016 08:06 AM, Khalid Aziz wrote:
> Top 4-bits of sparc64 virtual address are used for version tag only when
> a process has its PSTATE.mcde bit set and it is accessing a memory
> region that has ADI enabled on it (TTE.mcd set) and a version tag was
> set on the virtual address being accessed. These 4-bits retain their
> original semantics in all other cases.

OK, so this effectively reduces the address space of a process using the
feature.  Do we need to do anything explicit to keep an app from using
that address space?  Do we make sure the kernel doesn't place VMAs
there?  Do we respect mmap() hints that try to place memory there?

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

[Dave Hansen]

On 03/02/2016 12:39 PM, Khalid Aziz wrote:
> +long enable_sparc_adi(unsigned long addr, unsigned long len)
> +{
> + unsigned long end, pagemask;
> + int error;
> + struct vm_area_struct *vma, *vma2;
> + struct mm_struct *mm;
> +
> + if (!ADI_CAPABLE())
> + return -EINVAL;
...

This whole thing with the VMA splitting and so forth looks pretty darn
arch-independent.  Are you sure you need that much arch-specific code
for it, or can you share more of the generic VMA management code?

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

[Dave Hansen]

On 03/02/2016 12:39 PM, Khalid Aziz wrote:
> +long enable_sparc_adi(unsigned long addr, unsigned long len)
> +{
> + unsigned long end, pagemask;
> + int error;
> + struct vm_area_struct *vma, *vma2;
> + struct mm_struct *mm;
> +
> + if (!ADI_CAPABLE())
> + return -EINVAL;
...

This whole thing with the VMA splitting and so forth looks pretty darn
arch-independent.  Are you sure you need that much arch-specific code
for it, or can you share more of the generic VMA management code?

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

[Dave Hansen]

On 03/02/2016 12:39 PM, Khalid Aziz wrote:
> --- a/include/uapi/asm-generic/siginfo.h
> +++ b/include/uapi/asm-generic/siginfo.h
> @@ -206,7 +206,10 @@ typedef struct siginfo {
>  #define SEGV_MAPERR  (__SI_FAULT|1)  /* address not mapped to object */
>  #define SEGV_ACCERR  (__SI_FAULT|2)  /* invalid permissions for mapped 
> object */
>  #define SEGV_BNDERR  (__SI_FAULT|3)  /* failed address bound checks */
> -#define NSIGSEGV 3
> +#define SEGV_ACCADI  (__SI_FAULT|4)  /* ADI not enabled for mapped object */
> +#define SEGV_ADIDERR (__SI_FAULT|5)  /* Disrupting MCD error */
> +#define SEGV_ADIPERR (__SI_FAULT|6)  /* Precise MCD exception */
> +#define NSIGSEGV 6

FYI, this will conflict with code in -tip right now:

> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=mm/pkeys=cd0ea35ff5511cde299a61c21a95889b4a71464e

It's not a big deal to resolve, of course.

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

[Dave Hansen]

On 03/02/2016 12:39 PM, Khalid Aziz wrote:
> --- a/include/uapi/asm-generic/siginfo.h
> +++ b/include/uapi/asm-generic/siginfo.h
> @@ -206,7 +206,10 @@ typedef struct siginfo {
>  #define SEGV_MAPERR  (__SI_FAULT|1)  /* address not mapped to object */
>  #define SEGV_ACCERR  (__SI_FAULT|2)  /* invalid permissions for mapped 
> object */
>  #define SEGV_BNDERR  (__SI_FAULT|3)  /* failed address bound checks */
> -#define NSIGSEGV 3
> +#define SEGV_ACCADI  (__SI_FAULT|4)  /* ADI not enabled for mapped object */
> +#define SEGV_ADIDERR (__SI_FAULT|5)  /* Disrupting MCD error */
> +#define SEGV_ADIPERR (__SI_FAULT|6)  /* Precise MCD exception */
> +#define NSIGSEGV 6

FYI, this will conflict with code in -tip right now:

> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=mm/pkeys=cd0ea35ff5511cde299a61c21a95889b4a71464e

It's not a big deal to resolve, of course.

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700

> PR_GET_SPARC_ADICAPS

Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().

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

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700

> PR_GET_SPARC_ADICAPS

Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().