linux-kernel - Re: [PATCH V8 19/44] mm/pkeys: PKS Testing, add pks_mk

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: <Yg8v8XPyDTH4O2rr@iweiny-desk3>
Date:   Thu, 17 Feb 2022 21:34:41 -0800
From:   Ira Weiny <ira.weiny@...el.com>
To:     Dave Hansen <dave.hansen@...el.com>
Cc:     Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Rick Edgecombe <rick.p.edgecombe@...el.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH V8 19/44] mm/pkeys: PKS Testing, add pks_mk_*() tests

On Tue, Feb 01, 2022 at 09:45:03AM -0800, Dave Hansen wrote:
> On 1/27/22 09:54, ira.weiny@...el.com wrote:
> >  bool pks_test_callback(void)
> >  {
> > -	return false;
> > +	bool armed = (test_armed_key != 0);
> > +
> > +	if (armed) {
> > +		pks_mk_readwrite(test_armed_key);
> > +		fault_cnt++;
> > +	}
> > +
> > +	return armed;
> > +}
> 
> Where's the locking for all this?  I don't think we need anything fancy,
> but is there anything preventing the test from being started from
> multiple threads at the same time?  I think a simple global test mutex
> would probably suffice.

Good idea.  Generally I don't see that happening but it is good to be safe.

> 
> Also, pks_test_callback() needs at least a comment or two about what
> it's doing.

The previous patch which adds this call in the fault handler contains the
following comment which is in the final code:

/*
 * pks_test_callback() is called by the fault handler to indicate it saw a pkey
 * fault.
 *
 * NOTE: The callback is responsible for clearing any condition which would
 * cause the fault to re-trigger.
 */

Would you like more comments within the function?

> 
> Does this work if you have a test armed and then you get an unrelated
> PKS fault on another CPU?  I think this will disarm the test from the
> unrelated thread.

This code will detect a false fault.  But the other unrelated fault will work
correctly.

I've debated if the test code should use a specific fault callback...  :-/
That breaks my test which iterates all keys...  but would fix this problem.

Ira