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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 7 Nov 2022 12:23:24 +0000
From:   Szabolcs Nagy <szabolcs.nagy@....com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Joey Gouly <joey.gouly@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Lennart Poettering <lennart@...ttering.net>,
        Zbigniew Jędrzejewski-Szmek <zbyszek@...waw.pl>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Mark Brown <broonie@...nel.org>,
        Jeremy Linton <jeremy.linton@....com>,
        Topi Miettinen <toiwoton@...il.com>, linux-mm@...ck.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-abi-devel@...ts.sourceforge.net, nd@....com, shuah@...nel.org
Subject: Re: [PATCH v1 2/2] kselftest: vm: add tests for
 memory-deny-write-execute

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/