linux-kernel - Re: [PATCH] Prevent timer value 0 for MWAITX

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]

Message-ID: <f70daf94-be13-bad7-6c92-174891927e46@amd.com>
Date:   Thu, 27 Apr 2017 12:05:38 -0500
From:   "Natarajan, Janakarajan" <Janakarajan.Natarajan@....com>
To:     <linux-kernel@...r.kernel.org>, <x86@...nel.org>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H . Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH] Prevent timer value 0 for MWAITX

On 4/25/2017 4:44 PM, Janakarajan Natarajan wrote:
> This patch prevents the value 0 from being used for the MWAITX timer.
>
> Newer hardware has uncovered a bug in the software implementation of
> using MWAITX for the delay function. A value of 0 for the timer is meant
> to indicate that a timeout will not be used to exit MWAITX. On newer
> hardware this can result in MWAITX never returning, resulting in NMI
> soft lockup messages being printed. On older hardware, some of the other
> conditions under which MWAITX can exit masked this issue. The AMD APM
> does not currently document this and will be updated.
>
> Please refer to http://marc.info/?l=kvm&m=148950623231140 for
> information regarding NMI soft lockup messages on an AMD Ryzen 1800X.
> This has been root-caused as a 0 passed to MWAITX causing it to wait
> indefinitely.
>
> This change has the added benefit of avoiding the unnecessary setup of
> MONITORX/MWAITX when the delay value is zero.
>
> Cc: <stable@...r.kernel.org> # 4.4.x+
>
> Signed-off-by: Janakarajan Natarajan <Janakarajan.Natarajan@....com>
I know it's late in the cycle, but is there a possibility that this will 
make it into 4.11? It is a trivial fix and this issue is seen by folks 
outside.
> ---
>   arch/x86/lib/delay.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/arch/x86/lib/delay.c b/arch/x86/lib/delay.c
> index a8e91ae..29df077 100644
> --- a/arch/x86/lib/delay.c
> +++ b/arch/x86/lib/delay.c
> @@ -93,6 +93,13 @@ static void delay_mwaitx(unsigned long __loops)
>   {
>   	u64 start, end, delay, loops = __loops;
>   
> +	/*
> +	 * Timer value of 0 causes MWAITX to wait indefinitely, unless there
> +	 * is a store on the memory monitored by MONITORX.
> +	 */
> +	if (loops == 0)
> +		return;
> +
>   	start = rdtsc_ordered();
>   
>   	for (;;) {