Re: [PATCH v13 13/24] selftests/vm: pkey register should match shadow pkey
On Wed, Jun 20, 2018 at 07:53:57AM -0700, Dave Hansen wrote: > On 06/13/2018 05:45 PM, Ram Pai wrote: > > +++ b/tools/testing/selftests/vm/protection_keys.c > > @@ -916,10 +916,10 @@ void expected_pkey_fault(int pkey) > > pkey_assert(last_si_pkey == pkey); > > > > /* > > -* The signal handler shold have cleared out PKEY register to let the > > +* The signal handler should have cleared out pkey-register to let the > > * test program continue. We now have to restore it. > > */ > > - if (__read_pkey_reg() != 0) > > + if (__read_pkey_reg() != shadow_pkey_reg) > > pkey_assert(0); > > > > __write_pkey_reg(shadow_pkey_reg); > > I think this is wrong on x86. > > When we leave the signal handler, we zero out PKRU so that the faulting > instruction can continue, that's why we have the check against zero. > I'm actually kinda surprised this works. The code is modified to zero out only the violated key in the signal handler. Hence it works. Have verified it to work on x86 aswell. RP
Re: [PATCH v13 13/24] selftests/vm: pkey register should match shadow pkey
On 06/13/2018 05:45 PM, Ram Pai wrote: > +++ b/tools/testing/selftests/vm/protection_keys.c > @@ -916,10 +916,10 @@ void expected_pkey_fault(int pkey) > pkey_assert(last_si_pkey == pkey); > > /* > - * The signal handler shold have cleared out PKEY register to let the > + * The signal handler should have cleared out pkey-register to let the >* test program continue. We now have to restore it. >*/ > - if (__read_pkey_reg() != 0) > + if (__read_pkey_reg() != shadow_pkey_reg) > pkey_assert(0); > > __write_pkey_reg(shadow_pkey_reg); I think this is wrong on x86. When we leave the signal handler, we zero out PKRU so that the faulting instruction can continue, that's why we have the check against zero. I'm actually kinda surprised this works. Logically, this patch does: if (hardware != shadow) error(); hardware = shadow; That does not look right to me. What we want is: if (hardware != signal_return_pkey_reg) error(); hardware = shadow;
[PATCH v13 13/24] selftests/vm: pkey register should match shadow pkey
expected_pkey_fault() is comparing the contents of pkey register with 0. This may not be true all the time. There could be bits set by default by the architecture which can never be changed. Hence compare the value against shadow pkey register, which is supposed to track the bits accurately all throughout cc: Dave Hansen cc: Florian Weimer Signed-off-by: Ram Pai --- tools/testing/selftests/vm/protection_keys.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c index 9afe894..adcae4a 100644 --- a/tools/testing/selftests/vm/protection_keys.c +++ b/tools/testing/selftests/vm/protection_keys.c @@ -916,10 +916,10 @@ void expected_pkey_fault(int pkey) pkey_assert(last_si_pkey == pkey); /* -* The signal handler shold have cleared out PKEY register to let the +* The signal handler should have cleared out pkey-register to let the * test program continue. We now have to restore it. */ - if (__read_pkey_reg() != 0) + if (__read_pkey_reg() != shadow_pkey_reg) pkey_assert(0); __write_pkey_reg(shadow_pkey_reg); -- 1.7.1
