Re: [LTP] [PATCH] pkey01: Adding test for PKEY_DISABLE_EXECUTE

[Petr Vorel <pvorel@suse.cz>]

Hi Li,

> The pkey_test function now includes a code snippet to test execute
> permissions, ensuring proper handling of execution rights in memory
> protection keys.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

LGTM, thanks!

Few notes + tiny fixes below (which can be done before merge).

...
> +	pkey = pkey_alloc(0, PKEY_DISABLE_EXECUTE);
> +	if (pkey == -1) {
> +		if (errno == EINVAL) {
> +			tst_res(TINFO, "PKEY_DISABLE_EXECUTE hasn't implement.");
nit: maybe "PKEY_DISABLE_EXECUTE not implemented"

> +			execute_supported = 0;
> +		} else {
> +			tst_brk(TBROK | TERRNO, "pkey_alloc failed");
> +		}
> +	}
> +	pkey_free(pkey);
...

>  static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
>  {
>  	pid_t pid;
>  	char *buffer;
>  	int pkey, status;
>  	int fd = mpa->fd;
> +	size_t (*func)();
> +	size_t func_size = -1;
> +
> +	if (!execute_supported && (tc->access_rights == PKEY_DISABLE_EXECUTE))
> +		tst_brk(TCONF, "skip PKEY_DISABLE_EXECUTE test");
NOTE due to possible tst_brk() it'd be good to keep PKEY_DISABLE_EXECUTE as a
last test. Not sure if it's worth to add a comment (or replace with tst_res()
and return).

...
> +	if (mpa->prot == (PROT_READ | PROT_WRITE | PROT_EXEC)) {
> +		func_size = function_size((void (*)(void))function_size);
> +		memcpy(buffer, (void *)function_size, func_size);
> +	}
> +
>  	pkey = pkey_alloc(tc->flags, tc->access_rights);
>  	if (pkey == -1)
>  		tst_brk(TBROK | TERRNO, "pkey_alloc failed");
> @@ -169,6 +203,10 @@ static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
>  			tst_res(TFAIL | TERRNO,
>  				"Write buffer success, buffer[0] = %d", *buffer);
>  		break;
> +		case PKEY_DISABLE_EXECUTE:
> +			func = (size_t (*)())buffer;;
nit: double ; (please remove the last one).
> +			tst_res(TFAIL | TERRNO, "Excute buffer result = %lu", func(func));
s/%lu/%zi/ (otherwise it introduces warnings on 32bit)
nit: s/Excute/Execute/

> +		break;
>  		}
>  		exit(0);
>  	}
> @@ -193,10 +231,16 @@ static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
>  		tst_res(TPASS, "Write buffer success, buffer[0] = %d", *buffer);
>  	break;
>  	case PROT_READ | PROT_WRITE:
> -	case PROT_READ | PROT_WRITE | PROT_EXEC:
>  		*buffer = 'a';
>  		tst_res(TPASS, "Read & Write buffer success, buffer[0] = %d", *buffer);
>  	break;
> +	case PROT_READ | PROT_WRITE | PROT_EXEC:
> +		func = (size_t (*)())buffer;;
> +		if (func_size == func(func))
> +			tst_res(TPASS, "Execute buffer success, result = %lu", func_size);
s/%lu/%zi/
> +		else
> +			tst_res(TFAIL, "Execute buffer with unexpected result: %lu", func(func));
s/%lu/%zi/

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp