[PATCH] Fix tst-pkey.c pkey_alloc return checks and manual

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix tst-pkey.c pkey_alloc return checks and manual

Lucas A. M. Magalhaes
This test was failing in some powerpc systems as it was not checking
for ENOSPC return.

As said on the Linux man-pages and can be observed by the implementation
at mm/mprotect.c in the Linux Kernel source.  The syscall pkey_alloc can
return EINVAL or ENOSPC.  ENOSPC will indicate either that all keys are
in use or that the kernel does not support pkeys.
---
 manual/memory.texi                 | 4 ++++
 sysdeps/unix/sysv/linux/tst-pkey.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/manual/memory.texi b/manual/memory.texi
index b565dd69f2..aa5011e4f9 100644
--- a/manual/memory.texi
+++ b/manual/memory.texi
@@ -3288,6 +3288,10 @@ in which memory protection keys are disabled.
 
 @item ENOSPC
 All available protection keys already have been allocated.
+
+The system does not implement memory protection keys or runs in a mode
+in which memory protection keys are disabled.
+
 @end table
 @end deftypefun
 
diff --git a/sysdeps/unix/sysv/linux/tst-pkey.c b/sysdeps/unix/sysv/linux/tst-pkey.c
index 4c4fbae3ad..4ea1bc4f9a 100644
--- a/sysdeps/unix/sysv/linux/tst-pkey.c
+++ b/sysdeps/unix/sysv/linux/tst-pkey.c
@@ -197,6 +197,10 @@ do_test (void)
       if (errno == EINVAL)
         FAIL_UNSUPPORTED
           ("CPU does not support memory protection keys: %m");
+      if (errno == ENOSPC)
+        FAIL_UNSUPPORTED
+          ("no keys available or kernel does not support memory"
+           " protection keys");
       FAIL_EXIT1 ("pkey_alloc: %m");
     }
   TEST_COMPARE (pkey_get (keys[0]), 0);
--
2.20.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix tst-pkey.c pkey_alloc return checks and manual

Florian Weimer-5
* Lucas A. M. Magalhaes:

> This test was failing in some powerpc systems as it was not checking
> for ENOSPC return.
>
> As said on the Linux man-pages and can be observed by the implementation
> at mm/mprotect.c in the Linux Kernel source.  The syscall pkey_alloc can
> return EINVAL or ENOSPC.  ENOSPC will indicate either that all keys are
> in use or that the kernel does not support pkeys.
> ---
>  manual/memory.texi                 | 4 ++++
>  sysdeps/unix/sysv/linux/tst-pkey.c | 4 ++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/manual/memory.texi b/manual/memory.texi
> index b565dd69f2..aa5011e4f9 100644
> --- a/manual/memory.texi
> +++ b/manual/memory.texi
> @@ -3288,6 +3288,10 @@ in which memory protection keys are disabled.
>  
>  @item ENOSPC
>  All available protection keys already have been allocated.
> +
> +The system does not implement memory protection keys or runs in a mode
> +in which memory protection keys are disabled.
> +
>  @end table
>  @end deftypefun
>  
> diff --git a/sysdeps/unix/sysv/linux/tst-pkey.c b/sysdeps/unix/sysv/linux/tst-pkey.c
> index 4c4fbae3ad..4ea1bc4f9a 100644
> --- a/sysdeps/unix/sysv/linux/tst-pkey.c
> +++ b/sysdeps/unix/sysv/linux/tst-pkey.c
> @@ -197,6 +197,10 @@ do_test (void)
>        if (errno == EINVAL)
>          FAIL_UNSUPPORTED
>            ("CPU does not support memory protection keys: %m");
> +      if (errno == ENOSPC)
> +        FAIL_UNSUPPORTED
> +          ("no keys available or kernel does not support memory"
> +           " protection keys");
>        FAIL_EXIT1 ("pkey_alloc: %m");
>      }
>    TEST_COMPARE (pkey_get (keys[0]), 0);

Looks okay to me.

Siddhesh has to approve this as the release manager, though.

Thanks,
Florian

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix tst-pkey.c pkey_alloc return checks and manual

Siddhesh Poyarekar-8
On 16/01/20 7:18 pm, Florian Weimer wrote:
> Looks okay to me.
>
> Siddhesh has to approve this as the release manager, though.
>

This is OK for master.

Thanks,
Siddhesh
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix tst-pkey.c pkey_alloc return checks and manual

Gabriel F. T. Gomes-4
In reply to this post by Lucas A. M. Magalhaes
Hi, Lucas,

Thanks for doing this.  This failure has haunted my Debian systems for
a long time.

The patch looks good to me.  I only have a cosmetic suggestion.

Reviewed-by: Gabriel F. T. Gomes <[hidden email]>

On Thu, 16 Jan 2020, Lucas A. M. Magalhaes wrote:

> This test was failing in some powerpc systems as it was not checking
> for ENOSPC return.
>
> As said on the Linux man-pages and can be observed by the implementation
> at mm/mprotect.c in the Linux Kernel source.  The syscall pkey_alloc can
> return EINVAL or ENOSPC.  ENOSPC will indicate either that all keys are
> in use or that the kernel does not support pkeys.

Good commit message.

> --- a/manual/memory.texi
> +++ b/manual/memory.texi
> @@ -3288,6 +3288,10 @@ in which memory protection keys are disabled.
>  
>  @item ENOSPC
>  All available protection keys already have been allocated.
> +
> +The system does not implement memory protection keys or runs in a mode
> +in which memory protection keys are disabled.
> +

I think the wording at the commit message is better, because it makes
it clear that it's one situation or the other, so maybe:

  Either all available protection keys already have been allocated, or
  the system does not implement memory protection keys, or runs in a
  mode in which memory protection keys are disabled.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix tst-pkey.c pkey_alloc return checks and manual

Lucas A. M. Magalhaes
Quoting Gabriel F. T. Gomes (2020-01-16 10:57:53)

> Hi, Lucas,
>
> Thanks for doing this.  This failure has haunted my Debian systems for
> a long time.
>
> The patch looks good to me.  I only have a cosmetic suggestion.
>
> Reviewed-by: Gabriel F. T. Gomes <[hidden email]>
>
> On Thu, 16 Jan 2020, Lucas A. M. Magalhaes wrote:
>
> > This test was failing in some powerpc systems as it was not checking
> > for ENOSPC return.
> >
> > As said on the Linux man-pages and can be observed by the implementation
> > at mm/mprotect.c in the Linux Kernel source.  The syscall pkey_alloc can
> > return EINVAL or ENOSPC.  ENOSPC will indicate either that all keys are
> > in use or that the kernel does not support pkeys.
>
> Good commit message.
>
> > --- a/manual/memory.texi
> > +++ b/manual/memory.texi
> > @@ -3288,6 +3288,10 @@ in which memory protection keys are disabled.
> >  
> >  @item ENOSPC
> >  All available protection keys already have been allocated.
> > +
> > +The system does not implement memory protection keys or runs in a mode
> > +in which memory protection keys are disabled.
> > +
>
> I think the wording at the commit message is better, because it makes
> it clear that it's one situation or the other, so maybe:
>
>   Either all available protection keys already have been allocated, or
>   the system does not implement memory protection keys, or runs in a
>   mode in which memory protection keys are disabled.

Thanks Gabriel,

I actually agree with you, but I'm folowing the pattern of the EINVAL
explanation.  So I prefere leaving as it is. Maybe, in a furure patch,
change both return explanations in the way you suggest.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix tst-pkey.c pkey_alloc return checks and manual

Gabriel F. T. Gomes-4
On Thu, 16 Jan 2020, Lucas A. M. Magalhaes wrote:

> I actually agree with you, but I'm folowing the pattern of the EINVAL
> explanation.  So I prefere leaving as it is. Maybe, in a furure patch,
> change both return explanations in the way you suggest.

Fair enough. :)
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix tst-pkey.c pkey_alloc return checks and manual

Tulio Magno Quites Machado Filho-2
In reply to this post by Siddhesh Poyarekar-8
Siddhesh Poyarekar <[hidden email]> writes:

> On 16/01/20 7:18 pm, Florian Weimer wrote:
>> Looks okay to me.
>>
>> Siddhesh has to approve this as the release manager, though.
>
> This is OK for master.

Pushed as 70ba28f7ab2923d4e36ffc9d5d2e32357353b25c.

Thanks!

--
Tulio Magno