diff mbox series

[v13,10/24] selftests/vm: clear the bits in shadow reg when a pkey is freed.

Message ID 1528937115-10132-11-git-send-email-linuxram@us.ibm.com (mailing list archive)
State Superseded
Headers show
Series selftests, powerpc, x86 : Memory Protection Keys | expand

Commit Message

Ram Pai June 14, 2018, 12:45 a.m. UTC
When a key is freed, the  key  is  no  more  effective.
Clear the bits corresponding to the pkey in the shadow
register. Otherwise  it  will carry some spurious bits
which can trigger false-positive asserts.

cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 tools/testing/selftests/vm/protection_keys.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Comments

Dave Hansen June 20, 2018, 2:49 p.m. UTC | #1
On 06/13/2018 05:45 PM, Ram Pai wrote:
> When a key is freed, the  key  is  no  more  effective.
> Clear the bits corresponding to the pkey in the shadow
> register. Otherwise  it  will carry some spurious bits
> which can trigger false-positive asserts.
...--- a/tools/testing/selftests/vm/protection_keys.c
> +++ b/tools/testing/selftests/vm/protection_keys.c
> @@ -556,6 +556,9 @@ int alloc_pkey(void)
>  int sys_pkey_free(unsigned long pkey)
>  {
>  	int ret = syscall(SYS_pkey_free, pkey);
> +
> +	if (!ret)
> +		shadow_pkey_reg &= clear_pkey_flags(pkey, PKEY_DISABLE_ACCESS);
>  	dprintf1("%s(pkey=%ld) syscall ret: %d\n", __func__, pkey, ret);
>  	return ret;
>  }

This would be great code for an actual application.  But, I'm not
immediately convinced we want sane, kind behavior in our selftest.  x86
doesn't clear the hardware register at pkey_free, so wouldn't this cause
the shadow and the hardware register to diverge?
Ram Pai July 17, 2018, 4 p.m. UTC | #2
On Wed, Jun 20, 2018 at 07:49:31AM -0700, Dave Hansen wrote:
> On 06/13/2018 05:45 PM, Ram Pai wrote:
> > When a key is freed, the  key  is  no  more  effective.
> > Clear the bits corresponding to the pkey in the shadow
> > register. Otherwise  it  will carry some spurious bits
> > which can trigger false-positive asserts.
> ...--- a/tools/testing/selftests/vm/protection_keys.c
> > +++ b/tools/testing/selftests/vm/protection_keys.c
> > @@ -556,6 +556,9 @@ int alloc_pkey(void)
> >  int sys_pkey_free(unsigned long pkey)
> >  {
> >  	int ret = syscall(SYS_pkey_free, pkey);
> > +
> > +	if (!ret)
> > +		shadow_pkey_reg &= clear_pkey_flags(pkey, PKEY_DISABLE_ACCESS);
> >  	dprintf1("%s(pkey=%ld) syscall ret: %d\n", __func__, pkey, ret);
> >  	return ret;
> >  }
> 
> This would be great code for an actual application.  But, I'm not
> immediately convinced we want sane, kind behavior in our selftest.  x86
> doesn't clear the hardware register at pkey_free, so wouldn't this cause
> the shadow and the hardware register to diverge?

Have deleted the code in the newer version.

RP
diff mbox series

Patch

diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index da4f5d5..42a91c7 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -556,6 +556,9 @@  int alloc_pkey(void)
 int sys_pkey_free(unsigned long pkey)
 {
 	int ret = syscall(SYS_pkey_free, pkey);
+
+	if (!ret)
+		shadow_pkey_reg &= clear_pkey_flags(pkey, PKEY_DISABLE_ACCESS);
 	dprintf1("%s(pkey=%ld) syscall ret: %d\n", __func__, pkey, ret);
 	return ret;
 }