Patchwork [V2] Fix tst-pkey expectations on pkey_get

login
register
mail settings
Submitter Lucas A. M. Magalhaes
Date Feb. 13, 2020, 6:41 p.m.
Message ID <20200213184157.16778-1-lamm@linux.ibm.com>
Download mbox | patch
Permalink /patch/38050/
State New
Headers show

Comments

Lucas A. M. Magalhaes - Feb. 13, 2020, 6:41 p.m.
From the GNU LibC 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.

---

Hi,

This test still needs the patch on the link to work on POWER.
https://sourceware.org/ml/libc-alpha/2018-05/msg00760.html

in V2:
	- Fix signal handling expectations to accept x86 and POWER behavior.

 sysdeps/unix/sysv/linux/tst-pkey.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Florian Weimer - Feb. 14, 2020, 5:02 p.m.
* Lucas A. M. Magalhaes:

> From the GNU LibC 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.

“glibc manual” or “GNU C Library manual”.

> 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.

Please reference [BZ #23202] in the commit message.

> The pkey behavior during signal handling is different between x86 and
> POWER.  This change make the test compatible with both architectures.

> diff --git a/sysdeps/unix/sysv/linux/tst-pkey.c b/sysdeps/unix/sysv/linux/tst-pkey.c
> index 4ea1bc4f9a..ffa65cd5de 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];

Okay.

> @@ -118,7 +118,8 @@ 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;
>  }

Please add a comment explicitly describing the POWER and x86-64
behaviors.  Thanks.

Florian

Patch

diff --git a/sysdeps/unix/sysv/linux/tst-pkey.c b/sysdeps/unix/sysv/linux/tst-pkey.c
index 4ea1bc4f9a..ffa65cd5de 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];
 
@@ -118,7 +118,8 @@  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;
 }