Messages in this thread | | | Date | Fri, 28 Oct 2022 18:03:18 +0100 | From | Mark Brown <> | Subject | Re: [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] | |