lkml.org 
[lkml]   [2022]   [Oct]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v1 2/2] kselftest: vm: add tests for memory-deny-write-execute
On Wed, Oct 26, 2022 at 04:04:57PM +0100, Joey Gouly wrote:

> Add some tests to cover the new PR_SET_MDWE prctl.

Some comments below but they're all stylistic and let's not make perfect
be the enemy of the good here so

Reviewed-by: Mark Brown <broonie@kernel.org>

and we can iterate later rather than blocking anything on the testcase.

> +#ifdef __aarch64__
> +#define PROT_BTI 0x10 /* BTI guarded page */
> +#endif

We should get this from the kernel headers shouldn't we? We generally
rely on things getting pulled in from there rather than locally
defining.

> +#define TEST1 "mmap(PROT_WRITE | PROT_EXEC)\n"
> +#define TEST2 "mmap(PROT_WRITE); mprotect(PROT_EXEC)\n"
> +#define TEST3 "mmap(PROT_EXEC); mprotect(PROT_EXEC | PROT_READ)\n"
> +#define TEST4 "mmap(PROT_EXEC); mprotect(PROT_EXEC | PROT_BTI)\n"

> +int test1(int mdwe_enabled)
> +{

It feels like we could usefully make an array of

struct test {
int (*run)(bool mdwe_enabled);
char *name;
}

then we'd need fewer ifdefs, things could be more usefully named and
it'd be a bit easier to add new cases.

> +#ifdef __aarch64__
> + ksft_set_plan(12);
> +#else
> + ksft_set_plan(9);
> +#endif

That'd just be ksft_test_plan(3 * ARRAY_SIZE(tests).

> + // First run the tests without MDWE
> + test_result(test1(0), TEST1);
> + test_result(test2(0), TEST2);
> + test_result(test3(0), TEST3);
> +#ifdef __aarch64__
> + test_result(test4(0), TEST4);
> +#endif

and these calls to the tests would all be iterating over the array.
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2022-10-28 19:05    [W:0.114 / U:0.200 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site