linux-kernel - Re: [PATCH RFC V3 7/9] x86/entry: Preserve PKRS MSR across exceptions

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

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

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

Message-ID: <a3cb045e-3d27-eaca-c78d-d30d9a045e02@intel.com>
Date:   Wed, 14 Oct 2020 21:06:44 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Ira Weiny <ira.weiny@...el.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        Dan Williams <dan.j.williams@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Fenghua Yu <fenghua.yu@...el.com>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-nvdimm@...ts.01.org,
        linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
        linux-kselftest@...r.kernel.org
Subject: Re: [PATCH RFC V3 7/9] x86/entry: Preserve PKRS MSR across exceptions

On 10/14/20 8:46 PM, Ira Weiny wrote:
> On Tue, Oct 13, 2020 at 11:52:32AM -0700, Dave Hansen wrote:
>> On 10/9/20 12:42 PM, ira.weiny@...el.com wrote:
>>> @@ -341,6 +341,9 @@ noinstr void irqentry_enter(struct pt_regs *regs, irqentry_state_t *state)
>>>  	/* Use the combo lockdep/tracing function */
>>>  	trace_hardirqs_off();
>>>  	instrumentation_end();
>>> +
>>> +done:
>>> +	irq_save_pkrs(state);
>>>  }
>> One nit: This saves *and* sets PKRS.  It's not obvious from the call
>> here that PKRS is altered at this site.  Seems like there could be a
>> better name.
>>
>> Even if we did:
>>
>> 	irq_save_set_pkrs(state, INIT_VAL);
>>
>> It would probably compile down to the same thing, but be *really*
>> obvious what's going on.
> I suppose that is true.  But I think it is odd having a parameter which is the
> same for every call site.

Well, it depends on what you optimize for.  I'm trying to optimize for
the code being understood quickly the first time someone reads it.  To
me, that's more important than minimizing the number of function
parameters (which are essentially free).