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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

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

On 01/16/2017 09:39 PM, David Miller wrote:
> From: Khalid Aziz <khalid.aziz@...cle.com>
> Date: Wed, 11 Jan 2017 09:12:54 -0700
>
>> diff --git a/arch/sparc/kernel/mdesc.c b/arch/sparc/kernel/mdesc.c
>> index 8a6982d..68b03bf 100644
>> --- a/arch/sparc/kernel/mdesc.c
>> +++ b/arch/sparc/kernel/mdesc.c
>> @@ -20,6 +20,7 @@
>>  #include <asm/uaccess.h>
>>  #include <asm/oplib.h>
>>  #include <asm/smp.h>
>> +#include <asm/adi.h>
>>
>>  /* Unlike the OBP device tree, the machine description is a full-on
>>   * DAG.  An arbitrary number of ARCs are possible from one
>> @@ -1104,5 +1105,8 @@ void __init sun4v_mdesc_init(void)
>>
>>  	cur_mdesc = hp;
>>
>> +#ifdef CONFIG_SPARC64
>
> mdesc.c is only built on sparc64, this ifdef is superfluous.

Good point. I will fix it.

>
>> +/* Update the state of MCDPER register in current task's mm context before
>> + * dup so the dup'd task will inherit flags in this register correctly.
>> + * Current task may have updated flags since it started running.
>> + */
>> +int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
>> +{
>> +	if (adi_capable() && src->mm) {
>> +		register unsigned long tmp_mcdper;
>> +
>> +		__asm__ __volatile__(
>> +			".word 0x83438000\n\t"	/* rd %mcdper, %g1 */
>> +			"mov %%g1, %0\n\t"
>> +			: "=r" (tmp_mcdper)
>> +			:
>> +			: "g1");
>> +		src->mm->context.mcdper = tmp_mcdper;
>
> I don't like the idea of duplicating 'mm' state using the task struct
> copy.  Why do not the MM handling interfaces handle this properly?
>
> Maybe it means you've abstracted the ADI register handling in the
> wrong place.  Maybe it's a thread property which is "pushed" from
> the MM context.

I see what you are saying. This code updates mm->context.mcdper for the 
source thread with the current state of MCDPER since MCDPER can be 
changed by a userspace process any time. When userspace changes MCDPER, 
it is not saved into mm->context.mcdper until a context switch happens. 
This means during the timeslice for a thread, its mm->context.mcdper may 
not reflect the current value of MCDPER. Updating it ensures dup_mm() 
will copy the real current value of MCDPER into the newly forked thread. 
arch_dup_mmap() looks like a more appropriate place to do this. Do you 
agree?

Thanks,
Khalid