| Message ID | 1528937115-10132-20-git-send-email-linuxram@us.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
On 06/13/2018 05:45 PM, Ram Pai wrote: > +void test_read_of_access_disabled_region_with_page_already_mapped(int *ptr, > + u16 pkey) > +{ > + int ptr_contents; > + > + dprintf1("disabling access to PKEY[%02d], doing read @ %p\n", > + pkey, ptr); > + ptr_contents = read_ptr(ptr); > + dprintf1("reading ptr before disabling the read : %d\n", > + ptr_contents); > + read_pkey_reg(); > + pkey_access_deny(pkey); > + ptr_contents = read_ptr(ptr); > + dprintf1("*ptr: %d\n", ptr_contents); > + expected_pkey_fault(pkey); > +} Looks fine to me. I'm a bit surprised we didn't do this already, which is a good thing for this patch. FWIW, if you took patches like this and put them first, you could probably get it merged now. Yes, I know it would mean redoing some of the later code move and rename ones. -- 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:16:44AM -0700, Dave Hansen wrote: > On 06/13/2018 05:45 PM, Ram Pai wrote: > > +void test_read_of_access_disabled_region_with_page_already_mapped(int *ptr, > > + u16 pkey) > > +{ > > + int ptr_contents; > > + > > + dprintf1("disabling access to PKEY[%02d], doing read @ %p\n", > > + pkey, ptr); > > + ptr_contents = read_ptr(ptr); > > + dprintf1("reading ptr before disabling the read : %d\n", > > + ptr_contents); > > + read_pkey_reg(); > > + pkey_access_deny(pkey); > > + ptr_contents = read_ptr(ptr); > > + dprintf1("*ptr: %d\n", ptr_contents); > > + expected_pkey_fault(pkey); > > +} > > Looks fine to me. I'm a bit surprised we didn't do this already, which > is a good thing for this patch. > > FWIW, if you took patches like this and put them first, you could > probably get it merged now. Yes, I know it would mean redoing some of > the later code move and rename ones. I have incorporated almost all of your comments. But there are some comments that take some effort to implement. Shall we get the patches merged in the current form? This code has been sitting out for a while. In the current form its tested and works on powerpc and on x86, and incorporates about 95% of your suggestions. The rest I will take care as we go.
On 07/17/2018 09:13 AM, Ram Pai wrote: > I have incorporated almost all of your comments. But there are some > comments that take some effort to implement. Shall we get the patches > merged in the current form? This code has been sitting out for a while. > > In the current form its tested and works on powerpc and on x86, and > incorporates about 95% of your suggestions. The rest I will take care > as we go. What constitutes the remaining 5%? -- 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 Tue, Jul 17, 2018 at 10:56:08AM -0700, Dave Hansen wrote: > On 07/17/2018 09:13 AM, Ram Pai wrote: > > I have incorporated almost all of your comments. But there are some > > comments that take some effort to implement. Shall we get the patches > > merged in the current form? This code has been sitting out for a while. > > > > In the current form its tested and works on powerpc and on x86, and > > incorporates about 95% of your suggestions. The rest I will take care > > as we go. > > What constitutes the remaining 5%? Mostly your comments on code-organization in the signal handler. There are still some #if defined(__i386__) ..... Can be cleaned up and abstracted further. Also your questions on some of the code changes, the rationale for which is not obvious. Will help to spinkle in some descriptive comments there. Have fixed up a lot of codying style issues. But there could till be a few that may spew warning by checkpatch.pl. There are no functional issues AFAICT. RP -- 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
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c index e8ad970..04d0249 100644 --- a/tools/testing/selftests/vm/protection_keys.c +++ b/tools/testing/selftests/vm/protection_keys.c @@ -1024,6 +1024,24 @@ void test_read_of_access_disabled_region(int *ptr, u16 pkey) dprintf1("*ptr: %d\n", ptr_contents); expected_pkey_fault(pkey); } + +void test_read_of_access_disabled_region_with_page_already_mapped(int *ptr, + u16 pkey) +{ + int ptr_contents; + + dprintf1("disabling access to PKEY[%02d], doing read @ %p\n", + pkey, ptr); + ptr_contents = read_ptr(ptr); + dprintf1("reading ptr before disabling the read : %d\n", + ptr_contents); + read_pkey_reg(); + pkey_access_deny(pkey); + ptr_contents = read_ptr(ptr); + dprintf1("*ptr: %d\n", ptr_contents); + expected_pkey_fault(pkey); +} + void test_write_of_write_disabled_region(int *ptr, u16 pkey) { dprintf1("disabling write access to PKEY[%02d], doing write\n", pkey); @@ -1402,6 +1420,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey) void (*pkey_tests[])(int *ptr, u16 pkey) = { test_read_of_write_disabled_region, test_read_of_access_disabled_region, + test_read_of_access_disabled_region_with_page_already_mapped, test_write_of_write_disabled_region, test_write_of_access_disabled_region, test_kernel_write_of_access_disabled_region,