Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1351039 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-06 05:10 +0100
    Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 16:10 +0100
      Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Rob Gardner <rob.gardner@oracle.com> - 2016-03-07 16:40 +0100
        Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 16:50 +0100
        Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI) Andy Lutomirski <luto@amacapital.net> - 2016-03-07 16:50 +0100
          Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 17:10 +0100
            Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Dave Hansen <dave.hansen@linux.intel.com> - 2016-03-07 18:50 +0100
              Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI) Andy Lutomirski <luto@amacapital.net> - 2016-03-07 19:00 +0100
                Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Dave Hansen <dave.hansen@linux.intel.com> - 2016-03-07 19:20 +0100
                  Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 19:50 +0100
                    Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI) Andy Lutomirski <luto@amacapital.net> - 2016-03-07 20:00 +0100
                      Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-07 20:30 +0100
                        Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 20:50 +0100
                          Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Dave Hansen <dave.hansen@linux.intel.com> - 2016-03-07 23:50 +0100
                    Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Rob Gardner <rob.gardner@oracle.com> - 2016-03-08 02:40 +0100
              Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 22:10 +0100
                Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-08 21:00 +0100
                  Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-08 21:20 +0100
                    Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-08 21:30 +0100
                      Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-08 22:10 +0100
      Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-07 17:50 +0100
        Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 19:00 +0100
      Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-07 18:00 +0100
        Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI) Andy Lutomirski <luto@amacapital.net> - 2016-03-07 19:10 +0100
          Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 19:30 +0100
            Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI) Andy Lutomirski <luto@amacapital.net> - 2016-03-07 20:00 +0100
              Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-07 20:30 +0100
              Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 20:50 +0100
                Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI) Andy Lutomirski <luto@amacapital.net> - 2016-03-07 21:00 +0100
                  Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 21:50 +0100
                    Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-07 22:00 +0100
                      Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI) Andy Lutomirski <luto@amacapital.net> - 2016-03-07 22:10 +0100
                      Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 22:20 +0100
                      Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) James Morris <james.l.morris@oracle.com> - 2016-03-08 00:40 +0100
                  Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) James Morris <james.l.morris@oracle.com> - 2016-03-08 01:00 +0100
                    Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) James Morris <james.l.morris@oracle.com> - 2016-03-08 10:40 +0100
        Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 19:10 +0100
          Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Rob Gardner <rob.gardner@oracle.com> - 2016-03-07 19:20 +0100
            Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 19:30 +0100
              Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-07 20:20 +0100
                Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 22:40 +0100
                  Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-07 22:40 +0100
                    Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Rob Gardner <rob.gardner@oracle.com> - 2016-03-08 00:20 +0100
                      Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-08 05:20 +0100
                  Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Rob Gardner <rob.gardner@oracle.com> - 2016-03-08 00:20 +0100
                    Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-08 00:30 +0100
                Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-08 01:30 +0100
                  Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-08 05:30 +0100
              Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Rob Gardner <rob.gardner@oracle.com> - 2016-03-08 00:40 +0100
          Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-07 20:10 +0100
            Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 22:30 +0100
              Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) David Miller <davem@davemloft.net> - 2016-03-07 22:40 +0100
                Re: [PATCH v2] sparc64: Add support for Application Data Integrity  (ADI) Khalid Aziz <khalid.aziz@oracle.com> - 2016-03-07 23:40 +0100

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

#1351784

From: Khalid Aziz <khalid.aziz@oracle.com>
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.

[toc] | [prev] | [next] | [standalone]

#1351841

On 03/07/2016 09:45 AM, David Miller wrote:
> From: Khalid Aziz <khalid.aziz@oracle.com>
> 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

[toc] | [prev] | [next] | [standalone]

#1351800

From: Khalid Aziz <khalid.aziz@oracle.com>
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().

[toc] | [prev] | [next] | [standalone]

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

On Mon, Mar 7, 2016 at 10:04 AM, Khalid Aziz <khalid.aziz@oracle.com> wrote:
> On 03/07/2016 09:56 AM, David Miller wrote:
>>
>> From: Khalid Aziz <khalid.aziz@oracle.com>
>> 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.

[toc] | [prev] | [next] | [standalone]

#1351868

On 03/07/2016 11:08 AM, Andy Lutomirski wrote:
> On Mon, Mar 7, 2016 at 10:04 AM, Khalid Aziz <khalid.aziz@oracle.com> wrote:
>> On 03/07/2016 09:56 AM, David Miller wrote:
>>>
>>> From: Khalid Aziz <khalid.aziz@oracle.com>
>>> 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

[toc] | [prev] | [next] | [standalone]

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

On Mon, Mar 7, 2016 at 10:22 AM, Khalid Aziz <khalid.aziz@oracle.com> wrote:
> On 03/07/2016 11:08 AM, Andy Lutomirski wrote:
>>
>> On Mon, Mar 7, 2016 at 10:04 AM, Khalid Aziz <khalid.aziz@oracle.com>
>> wrote:
>>>
>>> On 03/07/2016 09:56 AM, David Miller wrote:
>>>>
>>>>
>>>> From: Khalid Aziz <khalid.aziz@oracle.com>
>>>> 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

[toc] | [prev] | [next] | [standalone]

#1351900

From: Andy Lutomirski <luto@amacapital.net>
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.

[toc] | [prev] | [next] | [standalone]

#1351930

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

Consider this scenario:

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

Does this make sense?

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

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

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

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

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

--
Khalid

[toc] | [prev] | [next] | [standalone]

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

On Mon, Mar 7, 2016 at 11:44 AM, Khalid Aziz <khalid.aziz@oracle.com> wrote:
> On 03/07/2016 11:49 AM, Andy Lutomirski wrote:
>>
>> On Mon, Mar 7, 2016 at 10:22 AM, Khalid Aziz <khalid.aziz@oracle.com>
>> wrote:
>>>
>>> No, it changes the tag associated with the virtual address for the
>>> caller.
>>> Physical page backing this virtual address is unaffected. Tag checking is
>>> done for virtual addresses. The one restriction where physical address is
>>> relevant is when two processes map the same physical page, they both have
>>> to
>>> use the same tag for the virtual addresses that map on to the shared
>>> physical pages.
>>
>>
>> Slow down, please.  *Why* do the tags for two different VAs that map
>> to the same PA have to match?  What goes wrong if they don't, and why
>> is requiring them to be the same a good idea?
>>
>
> Consider this scenario:
>
> 1. Process A creates a shm and attaches to it.
> 2. Process A fills shm with data it wants to share with only known
> processes. It enables ADI and sets tags on the shm.
> 3. Hacker triggers something like stack overflow on process A, exec's a new
> rogue binary and manages to attach to this shm. MMU knows tags were set on
> the virtual address mapping to the physical pages hosting the shm. If MMU
> does not require the rogue process to set the exact same tags on its mapping
> of the same shm, rogue process has defeated the ADI protection easily.
>
> Does this make sense?

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

[toc] | [prev] | [next] | [standalone]

#1351967

On 03/07/2016 12:54 PM, Andy Lutomirski wrote:
> On Mon, Mar 7, 2016 at 11:44 AM, Khalid Aziz <khalid.aziz@oracle.com> 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

[toc] | [prev] | [next] | [standalone]

#1351984

From: Khalid Aziz <khalid.aziz@oracle.com>
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.

[toc] | [prev] | [next] | [standalone]

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

On Mon, Mar 7, 2016 at 12:58 PM, David Miller <davem@davemloft.net> wrote:
> From: Khalid Aziz <khalid.aziz@oracle.com>
> 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

[toc] | [prev] | [next] | [standalone]

#1351995

On 03/07/2016 01:58 PM, David Miller wrote:
> From: Khalid Aziz <khalid.aziz@oracle.com>
> 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

[toc] | [prev] | [next] | [standalone]

#1352223

On 03/08/2016 07:58 AM, David Miller wrote:
> From: Khalid Aziz <khalid.aziz@oracle.com>
> 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

[toc] | [prev] | [next] | [standalone]

#1352311

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.

[toc] | [prev] | [next] | [standalone]

#1352798

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

[toc] | [prev] | [next] | [standalone]

#1351849

On 03/07/2016 09:56 AM, David Miller wrote:
> From: Khalid Aziz <khalid.aziz@oracle.com>
> 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

[toc] | [prev] | [next] | [standalone]

#1351856

On 03/07/2016 10:04 AM, Khalid Aziz wrote:
> On 03/07/2016 09:56 AM, David Miller wrote:
>> From: Khalid Aziz <khalid.aziz@oracle.com>
>> 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

[toc] | [prev] | [next] | [standalone]

#1351862

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 <khalid.aziz@oracle.com>
>>> 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

[toc] | [prev] | [next] | [standalone]

#1351899

From: Khalid Aziz <khalid.aziz@oracle.com>
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.

[toc] | [prev] | [next] | [standalone]

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web