| Message ID | 1528937115-10132-23-git-send-email-linuxram@us.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
On 06/13/2018 05:45 PM, Ram Pai wrote: > Generally the signal handler restores the state of the pkey register > before returning. However there are times when the read/write operation > can legitamely fail without invoking the signal handler. Eg: A > sys_read() operaton to a write-protected page should be disallowed. In > such a case the state of the pkey register is not restored to its > original state. The test case is responsible for restoring the key > register state to its original value. Seems fragile. Can't we just do this in common code? We could just loop through and restore the default permissions. That seems much more resistant to a bad test case. -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 20, 2018 at 08:20:22AM -0700, Dave Hansen wrote: > On 06/13/2018 05:45 PM, Ram Pai wrote: > > Generally the signal handler restores the state of the pkey register > > before returning. However there are times when the read/write operation > > can legitamely fail without invoking the signal handler. Eg: A > > sys_read() operaton to a write-protected page should be disallowed. In > > such a case the state of the pkey register is not restored to its > > original state. The test case is responsible for restoring the key > > register state to its original value. > > Seems fragile. Can't we just do this in common code? We could just > loop through and restore the default permissions. That seems much more > resistant to a bad test case. Yes. done. fixed it the way you suggested in the new version.
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c index caf634e..b5a9e6c 100644 --- a/tools/testing/selftests/vm/protection_keys.c +++ b/tools/testing/selftests/vm/protection_keys.c @@ -1011,6 +1011,7 @@ void test_read_of_write_disabled_region(int *ptr, u16 pkey) ptr_contents = read_ptr(ptr); dprintf1("*ptr: %d\n", ptr_contents); dprintf1("\n"); + pkey_write_allow(pkey); } void test_read_of_access_disabled_region(int *ptr, u16 pkey) { @@ -1090,6 +1091,7 @@ void test_kernel_write_of_access_disabled_region(int *ptr, u16 pkey) ret = read(test_fd, ptr, 1); dprintf1("read ret: %d\n", ret); pkey_assert(ret); + pkey_access_allow(pkey); } void test_kernel_write_of_write_disabled_region(int *ptr, u16 pkey) { @@ -1102,6 +1104,7 @@ void test_kernel_write_of_write_disabled_region(int *ptr, u16 pkey) if (ret < 0 && (DEBUG_LEVEL > 0)) perror("verbose read result (OK for this to be bad)"); pkey_assert(ret); + pkey_write_allow(pkey); } void test_kernel_gup_of_access_disabled_region(int *ptr, u16 pkey) @@ -1121,6 +1124,7 @@ void test_kernel_gup_of_access_disabled_region(int *ptr, u16 pkey) vmsplice_ret = vmsplice(pipe_fds[1], &iov, 1, SPLICE_F_GIFT); dprintf1("vmsplice() ret: %d\n", vmsplice_ret); pkey_assert(vmsplice_ret == -1); + pkey_access_allow(pkey); close(pipe_fds[0]); close(pipe_fds[1]); @@ -1141,6 +1145,7 @@ void test_kernel_gup_write_to_write_disabled_region(int *ptr, u16 pkey) if (DEBUG_LEVEL > 0) perror("futex"); dprintf1("futex() ret: %d\n", futex_ret); + pkey_write_allow(pkey); } /* Assumes that all pkeys other than 'pkey' are unallocated */
Generally the signal handler restores the state of the pkey register before returning. However there are times when the read/write operation can legitamely fail without invoking the signal handler. Eg: A sys_read() operaton to a write-protected page should be disallowed. In such a case the state of the pkey register is not restored to its original state. The test case is responsible for restoring the key register state to its original value. 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 | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)