Message ID | 20200217120952.5219-1-lamm@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | [v4] Fix tst-pkey expectations on pkey_get [BZ #23202] | expand |
* Lucas A. M. Magalhaes: > diff --git a/sysdeps/unix/sysv/linux/tst-pkey.c b/sysdeps/unix/sysv/linux/tst-pkey.c > index 4ea1bc4f9a..cba40c73de 100644 > --- a/sysdeps/unix/sysv/linux/tst-pkey.c > +++ b/sysdeps/unix/sysv/linux/tst-pkey.c > @@ -37,7 +37,7 @@ static pthread_barrier_t barrier; > > /* The keys used for testing. These have been allocated with access > rights set based on their array index. */ > -enum { key_count = 4 }; > +enum { key_count = 3 }; > static int keys[key_count]; > static volatile int *pages[key_count]; > > @@ -111,14 +111,16 @@ check_page_access (int page, bool write) > } > > static volatile sig_atomic_t sigusr1_handler_ran; > - > -/* Used to check that access is revoked in signal handlers. */ > +/* Used to check the behavior in signal handlers. In x86 all access are > + revoked during signal handling. In PowerPC the key permissions are > + inherited by the interrupted thread. This test accept both approaches. */ > static void > sigusr1_handler (int signum) > { > TEST_COMPARE (signum, SIGUSR1); > for (int i = 0; i < key_count; ++i) > - TEST_COMPARE (pkey_get (keys[i]), PKEY_DISABLE_ACCESS); > + TEST_VERIFY (pkey_get (keys[i]) == PKEY_DISABLE_ACCESS > + || pkey_get (keys[i]) == i); > sigusr1_handler_ran = 1; > } This version looks okay to me. Thanks. Florian
"Lucas A. M. Magalhaes" <lamm@linux.ibm.com> writes: > From the GNU C Library manual the pkey_set can receive a combination of > PKEY_DISABLE_WRITE and PKEY_DISABLE_ACCESS. However PKEY_DISABLE_ACCESS > is more restrictive than PKEY_DISABLE_WRITE and includes its behavior. > > The test expects that after setting > (PKEY_DISABLE_WRITE|PKEY_DISABLE_ACCESS) pkey_get should return the > same. This may not be true as PKEY_DISABLE_ACCESS will succeed in > describe the state of the key in this case. > > The pkey behavior during signal handling is different between x86 and > POWER. This change make the test compatible with both architectures. The patch LGTM. This test is now passing on Linux >= 5.0, but it still fails on Linux <= 4.18 because of issues in the kernel. I'm not sure if 4.19 should work. Old kernels do return valid data: [pid 56757] pkey_alloc(0, 0 <unfinished ...> [pid 56758] set_robust_list(0x7fff8994f260, 24 <unfinished ...> [pid 56757] <... pkey_alloc resumed> ) = 2 [pid 56758] <... set_robust_list resumed> ) = 0 [pid 56757] pkey_alloc(0, PKEY_DISABLE_ACCESS <unfinished ...> [pid 56758] futex(0x100208a4, FUTEX_WAIT_PRIVATE, 0, NULL <unfinished ...> [pid 56757] <... pkey_alloc resumed> ) = 3 [pid 56757] pkey_alloc(0, PKEY_DISABLE_WRITE) = 4 [pid 56757] mmap(NULL, 65536, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fff89130000 [pid 56757] pkey_mprotect(0x7fff89130000, 65536, PROT_READ|PROT_WRITE, 2) = 0 [pid 56757] mmap(NULL, 65536, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fff89120000 [pid 56757] pkey_mprotect(0x7fff89120000, 65536, PROT_READ|PROT_WRITE, 3) = 0 [pid 56757] mmap(NULL, 65536, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fff89110000 [pid 56757] pkey_mprotect(0x7fff89110000, 65536, PROT_READ|PROT_WRITE, 4) = 0 [pid 56757] futex(0x100208a4, FUTEX_WAKE_PRIVATE, 2147483647) = 1 [pid 56758] <... futex resumed> ) = 0 [pid 56757] futex(0x7fff8994f250, FUTEX_WAIT, 56758, NULL <unfinished ...> [pid 56758] mmap(NULL, 134217728, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = 0x7fff81110000 [pid 56758] munmap(0x7fff81110000, 49217536) = 0 [pid 56758] munmap(0x7fff88000000, 17891328) = 0 [pid 56758] mprotect(0x7fff84000000, 196608, PROT_READ|PROT_WRITE) = 0 [pid 56758] rt_sigprocmask(SIG_BLOCK, NULL, [], 8) = 0 [pid 56758] rt_sigaction(SIGSEGV, {sa_handler=0x1001fca8, sa_mask=[], sa_flags=SA_RESETHAND|SA_SIGINFO}, NULL, 8) = 0 [pid 56758] rt_sigaction(SIGSEGV, {sa_handler=SIG_DFL, sa_mask=[SEGV], sa_flags=SA_RESTART}, {sa_handler=0x1001fca8, sa_mask=[], sa_flags=SA_RESETHAND|SA_SIGINFO}, 8) = 0 [pid 56758] write(1, "error: ../sysdeps/unix/sysv/linu"..., 90error: ../sysdeps/unix/sysv/linux/tst-pkey.c:161: not true: !check_page_access (i, false) ) = 90 Anyway, pushed as 8d42bf859a289944749d9f978c076cd318119867. Thanks!
diff --git a/sysdeps/unix/sysv/linux/tst-pkey.c b/sysdeps/unix/sysv/linux/tst-pkey.c index 4ea1bc4f9a..cba40c73de 100644 --- a/sysdeps/unix/sysv/linux/tst-pkey.c +++ b/sysdeps/unix/sysv/linux/tst-pkey.c @@ -37,7 +37,7 @@ static pthread_barrier_t barrier; /* The keys used for testing. These have been allocated with access rights set based on their array index. */ -enum { key_count = 4 }; +enum { key_count = 3 }; static int keys[key_count]; static volatile int *pages[key_count]; @@ -111,14 +111,16 @@ check_page_access (int page, bool write) } static volatile sig_atomic_t sigusr1_handler_ran; - -/* Used to check that access is revoked in signal handlers. */ +/* Used to check the behavior in signal handlers. In x86 all access are + revoked during signal handling. In PowerPC the key permissions are + inherited by the interrupted thread. This test accept both approaches. */ static void sigusr1_handler (int signum) { TEST_COMPARE (signum, SIGUSR1); for (int i = 0; i < key_count; ++i) - TEST_COMPARE (pkey_get (keys[i]), PKEY_DISABLE_ACCESS); + TEST_VERIFY (pkey_get (keys[i]) == PKEY_DISABLE_ACCESS + || pkey_get (keys[i]) == i); sigusr1_handler_ran = 1; }