Re: [PATCH 7/9] pkeys.7: Update the example to match glibc

["Alejandro Colomar (man-pages)" <alx.manpages@xxxxxxxxx>]

Hi Stephen,

On 1/7/22 17:46, Stephen Kitt wrote:
> glibc 2.27 introduced support for the pkeys functions, but the glibc
> versions don't match those declared in the example. Update the example
> to match glibc, and avoid declaring the functions if glibc is new
> enough. >
> Signed-off-by: Stephen Kitt <steve@xxxxxxx>


There are a few problems with the prototypes.


alx@ady1:~/src/gnu/glibc$ grep_glibc_prototype wrpkru
alx@ady1:~/src/gnu/glibc$ grep -rn define.wrpkru
alx@ady1:~/src/gnu/glibc$ grep_glibc_prototype pkey_set
60:int pkey_set (int __key, unsigned int __access_rights) __THROW;
alx@ady1:~/src/gnu/glibc$ grep_glibc_prototype pkey_mprotect
72:int pkey_mprotect (void *__addr, size_t __len, int __prot, int 
__pkey) __THROW;
alx@ady1:~/src/gnu/glibc$ grep_glibc_prototype pkey_alloc
56:int pkey_alloc (unsigned int __flags, unsigned int __access_rights) 
__THROW;
alx@ady1:~/src/gnu/glibc$ grep_glibc_prototype pkey_free
68:int pkey_free (int __key) __THROW;


As you see above, I couldn't find wrpkru().  Are you sure it exists in 
glibc?

pkey_mprotect(3) uses 'int' instead of 'unsigned long'.  Would you mind 
fixind that one too?

pkey_set(3) uses 'unsigned int' instead of 'unsigned long'.  Please fix 
that one.

pkey_free(3) uses 'int' instead of 'unsigned long'.  Would you mind 
fixing that one too?

BTW, I need to modify grep_glibc_prototype() so that it always prints 
the file name, even if only one file is passed to grep (adding /dev/null 
to the file list).


A part from that, I prefer EXAMPLES to be as simple as possible, so I'd 
do 2 patches.  One to match the definitions to the glibc ones, and then 
one commit removing old code, assuming glibc is new enough.  Would you 
mind sending a subsequent patch to remove everything under #if ... #endif?


Thanks,

Alex

> ---
>   man7/pkeys.7 | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/man7/pkeys.7 b/man7/pkeys.7
> index 73ddcdc43..480ff21d4 100644
> --- a/man7/pkeys.7
> +++ b/man7/pkeys.7
> @@ -186,8 +186,10 @@ Segmentation fault (core dumped)
>   #include <unistd.h>
>   #include <sys/syscall.h>
>   #include <stdio.h>
> +#include <stdlib.h>
>   #include <sys/mman.h>
>   
> +#if __GLIBC__ < 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ < 27)
>   static inline void
>   wrpkru(unsigned int pkru)
>   {
> @@ -200,7 +202,7 @@ wrpkru(unsigned int pkru)
>   }
>   
>   int
> -pkey_set(int pkey, unsigned long rights, unsigned long flags)
> +pkey_set(int pkey, unsigned long rights)
>   {
>       unsigned int pkru = (rights << (2 * pkey));
>       return wrpkru(pkru);
> @@ -214,9 +216,9 @@ pkey_mprotect(void *ptr, size_t size, unsigned long orig_prot,
>   }
>   
>   int
> -pkey_alloc(void)
> +pkey_alloc(unsigned int flags, unsigned int rights)
>   {
> -    return syscall(SYS_pkey_alloc, 0, 0);
> +    return syscall(SYS_pkey_alloc, flags, rights);
>   }
>   
>   int
> @@ -224,6 +226,7 @@ pkey_free(unsigned long pkey)
>   {
>       return syscall(SYS_pkey_free, pkey);
>   }
> +#endif
>   
>   #define errExit(msg)    do { perror(msg); exit(EXIT_FAILURE); \e
>                              } while (0)
> @@ -252,7 +255,7 @@ main(void)
>       /*
>        * Allocate a protection key:
>        */
> -    pkey = pkey_alloc();
> +    pkey = pkey_alloc(0, 0);
>       if (pkey == \-1)
>           errExit("pkey_alloc");
>   
> @@ -260,7 +263,7 @@ main(void)
>        * Disable access to any memory with "pkey" set,
>        * even though there is none right now.
>        */
> -    status = pkey_set(pkey, PKEY_DISABLE_ACCESS, 0);
> +    status = pkey_set(pkey, PKEY_DISABLE_ACCESS);
>       if (status)
>           errExit("pkey_set");
>   

--
Alejandro Colomar
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/