linux-kernel - Re: [PATCH RFC V2 02/17] x86/fpu: Refactor arch_set_user_pkey

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: <1f9586df-cabb-c743-1eeb-f44c73459fb5@intel.com>
Date:   Fri, 17 Jul 2020 15:36:12 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Peter Zijlstra <peterz@...radead.org>, 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>,
        Fenghua Yu <fenghua.yu@...el.com>, x86@...nel.org,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Vishal Verma <vishal.l.verma@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        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 V2 02/17] x86/fpu: Refactor
 arch_set_user_pkey_access() for PKS support

On 7/17/20 1:54 AM, Peter Zijlstra wrote:
> This is unbelievable junk...

Ouch!

This is from the original user pkeys implementation.

> How about something like:
> 
> u32 update_pkey_reg(u32 pk_reg, int pkey, unsigned int flags)
> {
> 	int pkey_shift = pkey * PKR_BITS_PER_PKEY;
> 
> 	pk_reg &= ~(((1 << PKR_BITS_PER_PKEY) - 1) << pkey_shift);
> 
> 	if (flags & PKEY_DISABLE_ACCESS)
> 		pk_reg |= PKR_AD_BIT << pkey_shift;
> 	if (flags & PKEY_DISABLE_WRITE)
> 		pk_reg |= PKR_WD_BIT << pkey_shift;
> 
> 	return pk_reg;
> }
> 
> Then we at least have a little clue wtf the thing does.. Yes I started
> with a rename and then got annoyed at the implementation too.

That's fine, if some comments get added.  It looks correct to me but
probably compiles down to pretty much the same thing as what was there.
 FWIW, I prefer the explicit masking off of two bit values to implicit
masking off with a mask generated from PKR_BITS_PER_PKEY.  It's
certainly more compact, but I usually don't fret over the lines of code.