Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Andy Lutomirski <luto@amacapital.net>
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH] lkdtm/bugs: Avoid ifdefs for DOUBLE_FAULT
Date:   Wed, 27 Nov 2019 13:01:40 -0800
Message-Id: <E359D543-FB4E-4EF5-85E6-40057B57C58F@amacapital.net>
References: <201911271118.FCC2D04F@keescook>
Cc:     Ingo Molnar <mingo@kernel.org>, Andy Lutomirski <luto@kernel.org>,
        x86 <x86@kernel.org>, linux-kernel@vger.kernel.org
In-Reply-To: <201911271118.FCC2D04F@keescook>
To:     Kees Cook <keescook@chromium.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Nov 27, 2019, at 11:19 AM, Kees Cook <keescook@chromium.org> wrote:
>=20
> =EF=BB=BFLKDTM test visibility shouldn't change, so remove the ifdefs on
> DOUBLE_FAULT and make sure test failure doesn't crash the system.
>=20
> Link: https://lore.kernel.org/lkml/20191127184837.GA35982@gmail.com
> Fixes: b09511c253e5 ("lkdtm: Add a DOUBLE_FAULT crash type on x86")
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> applies on top of tip/x86/urgent
> ---
> drivers/misc/lkdtm/bugs.c  | 8 +++++---
> drivers/misc/lkdtm/core.c  | 4 +---
> drivers/misc/lkdtm/lkdtm.h | 2 --
> 3 files changed, 6 insertions(+), 8 deletions(-)
>=20
> diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
> index a4fdad04809a..22f5293414cc 100644
> --- a/drivers/misc/lkdtm/bugs.c
> +++ b/drivers/misc/lkdtm/bugs.c
> @@ -342,9 +342,9 @@ void lkdtm_UNSET_SMEP(void)
> #endif
> }
>=20
> -#ifdef CONFIG_X86_32
> void lkdtm_DOUBLE_FAULT(void)
> {
> +#ifdef CONFIG_X86_32
>    /*
>     * Trigger #DF by setting the stack limit to zero.  This clobbers
>     * a GDT TLS slot, which is okay because the current task will die
> @@ -373,6 +373,8 @@ void lkdtm_DOUBLE_FAULT(void)
>    asm volatile ("movw %0, %%ss; addl $0, (%%esp)" ::
>              "r" ((unsigned short)(GDT_ENTRY_TLS_MIN << 3)));
>=20
> -    panic("tried to double fault but didn't die\n");
> -}
> +    pr_err("FAIL: tried to double fault but didn't die!\n");
> +#else
> +    pr_err("FAIL: this test is only available on 32-bit x86.\n");
> #endif
> +}

I=E2=80=99m not familiar with the userspace tooling, but this seems unfortun=
ate. The first FAIL is =E2=80=9Cthe test case screwed up, and it=E2=80=99s a=
 bug.=E2=80=9D  The second FAIL is =E2=80=9Cnot applicable to this system.=E2=
=80=9D


ISTM simply not exposing the test on systems that don=E2=80=99t support make=
s sense. Can you clarify?=