Re: [PATCH v1 2/2] kselftest: vm: add tests for memory-deny-write-execute

[Szabolcs Nagy <szabolcs.nagy@arm.com>]

The 10/28/2022 13:16, Kees Cook wrote:
> +++ b/tools/testing/selftests/vm/mdwe_test.c
> @@ -0,0 +1,201 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#ifdef __aarch64__
> +#include <asm/hwcap.h>
> +#endif
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/auxv.h>
> +#include <sys/mman.h>
> +#include <sys/prctl.h>
> +#include <sys/wait.h>
> +#include <unistd.h>
> +
> +#include <linux/prctl.h>
> +
> +#include "../kselftest_harness.h"
> +
> +#define PR_SET_MDWE                     65
> +# define PR_MDWE_FLAG_MMAP              1
> +
> +#define PR_GET_MDWE                     66
> +
> +#ifdef __aarch64__
> +# define PROT_BTI	0x10            /* BTI guarded page */
> +#else
> +# define PROT_BTI	0
> +#endif
> +
> +TEST(prctl_flags)
> +{
> +	EXPECT_LT(prctl(PR_SET_MDWE, 7, 0, 0, 0), 0);
> +	EXPECT_LT(prctl(PR_SET_MDWE, 0, 7, 0, 0), 0);
> +	EXPECT_LT(prctl(PR_SET_MDWE, 0, 0, 7, 0), 0);
> +	EXPECT_LT(prctl(PR_SET_MDWE, 0, 0, 0, 7), 0);

note that prctl is declared as

  int prctl(int, ...);

and all 4 arguments are documented to be unsigned long in
the linux man pages (even though some are pointers: this
is already a problem for the libc as it does not know if it
should use va_arg(ap, unsigned long) or va_arg(ap, void *),
in practice the call abi rules are the same for those on
linux, so either works unless the compiler deliberately
breaks the code due to the type mismatch ub).

passing an int where an unsigned long is needed is wrong: it
breaks va_arg rules on the c language level (posix rules too)
but more importantly it breaks abi rules: on most LP64 abis
it is not required to be signextended so arbitrary top 32bits
may be passed down.

so e.g.

  prctl(option, 0, 0, 0, 0);

should be written as

  prctl(option, 0L, 0L, 0L, 0L);

or similar (int signedness does not matter according to c
rules), otherwise non-zero top bits may be passed that the
kernel has to ignore, which it currently does not always do.

ideally the kernel updated all the prctl arg macros to have
type long or unsigned long. or explicitly masked out the top
bits when it only uses an int.

see my related rant at
https://lore.kernel.org/linux-api/Y1%2FDS6uoWP7OSkmd@arm.com/