Re: [PATCH] selftests: powerpc: Add test for execute-disabled pkeys

[Sandipan Das <sandipan@linux.ibm.com>]

Hi Michael,

On 26/05/20 6:05 pm, Michael Ellerman wrote:
> [...]
>> +
>> +/* Override definitions as they might be inconsistent */
>> +#undef PKEY_DISABLE_ACCESS
>> +#define PKEY_DISABLE_ACCESS	0x3
> 
> Why would they be inconsistent?
> 

The definition in sys/mman.h still uses the value specific to
Intel's implementation i.e. 1, when this should have been 3
for powerpc. I have seen this on Ubuntu 18.04 and 20.04.

> 
>> +/* Older distros might not define this */
>> +#ifndef SEGV_PKUERR
>> +#define SEGV_PKUERR	4
>> +#endif
> ...
>> +
>> +	/* Restore permissions in order to continue */
>> +	switch (fcode) {
>> +	case SEGV_ACCERR:
>> +		if (mprotect(insns, pgsize, PROT_READ | PROT_WRITE)) {
>> +			perror("mprotect");
>> +			goto fail;
>> +		}
>> +		break;
>> +	case SEGV_PKUERR:
>> +		if (sinfo->si_pkey != fpkey)
>> +			goto fail;
> 
> This doesn't compile on older distros, eg Ubuntu 16.04:
> 
>   pkey_exec_prot.c: In function 'segv_handler':
>   pkey_exec_prot.c:121:12: error: 'siginfo_t {aka struct <anonymous>}' has no member named 'si_pkey'
>      if (sinfo->si_pkey != fpkey)
>               ^
>   pkey_exec_prot.c:151:24: error: 'siginfo_t {aka struct <anonymous>}' has no member named 'si_pkey'
>      pkey_set_rights(sinfo->si_pkey, 0);
>                           ^
>   ../../lib.mk:142: recipe for target '/output/kselftest/powerpc/mm/pkey_exec_prot' failed
> 

Thanks for reporting this.

> 
> I think a reasonable solution is to use the absence of SEGV_PKUERR to
> basically turn the whole test into a nop at build time, eg:
> 
> diff --git a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
> index b346ad205e68..218257b89fbb 100644
> --- a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
> +++ b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
> @@ -30,9 +30,7 @@
>  #define PKEY_DISABLE_EXECUTE   0x4
> 
>  /* Older distros might not define this */
> -#ifndef SEGV_PKUERR
> -#define SEGV_PKUERR    4
> -#endif
> +#ifdef SEGV_PKUERR
> 
>  #define SYS_pkey_mprotect      386
>  #define SYS_pkey_alloc         384
> @@ -319,6 +317,13 @@ static int test(void)
> 
>         return 0;
>  }
> +#else
> +static int test(void)
> +{
> +       printf("Test built with old libc lacking pkey support.\n");
> +       SKIP_IF(true);
> +}
> +#endif /* SEGV_PKUERR */
> 
>  int main(void)
>  {
> 
> 

Or can I use this from the pkey tests under selftests/vm?

static inline u32 *siginfo_get_pkey_ptr(siginfo_t *si)
{
#ifdef si_pkey
	return &si->si_pkey;
#else
	return (u32 *)(((u8 *)si) + si_pkey_offset);
#endif
}

Where si_pkey_offset is 0x20 for powerpc.


- Sandipan