linux-kernel - Re: [PATCH 4.19 112/191] x86/nmi: Remove irq_work from the long duration NMI handler

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: <20200221222252.GA14067@amd>
Date:   Fri, 21 Feb 2020 23:22:53 +0100
From:   Pavel Machek <pavel@...x.de>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Changbin Du <changbin.du@...il.com>,
        Borislav Petkov <bp@...e.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 4.19 112/191] x86/nmi: Remove irq_work from the long
 duration NMI handler

Hi!

On Fri 2020-02-21 08:41:25, Greg Kroah-Hartman wrote:
> From: Changbin Du <changbin.du@...il.com>
> 
> [ Upstream commit 248ed51048c40d36728e70914e38bffd7821da57 ]
> 
> First, printk() is NMI-context safe now since the safe printk() has been
> implemented and it already has an irq_work to make NMI-context safe.
> 
> Second, this NMI irq_work actually does not work if a NMI handler causes
> panic by watchdog timeout. It has no chance to run in such case, while
> the safe printk() will flush its per-cpu buffers before panicking.
> 
> While at it, repurpose the irq_work callback into a function which
> concentrates the NMI duration checking and makes the code easier to
> follow.

I know there were printk() changes recently, but are they all in 4.19?

Does this actually fix any bug in 4.19?

Best regards,
								Pavel
								
> diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> index 086cf1d1d71d8..0f8b9b900b0e7 100644
> --- a/arch/x86/kernel/nmi.c
> +++ b/arch/x86/kernel/nmi.c
> @@ -102,18 +102,22 @@ static int __init nmi_warning_debugfs(void)
>  }
>  fs_initcall(nmi_warning_debugfs);
>  
> -static void nmi_max_handler(struct irq_work *w)
> +static void nmi_check_duration(struct nmiaction *action, u64 duration)
>  {
> -	struct nmiaction *a = container_of(w, struct nmiaction, irq_work);
> +	u64 whole_msecs = READ_ONCE(action->max_duration);
>  	int remainder_ns, decimal_msecs;
> -	u64 whole_msecs = READ_ONCE(a->max_duration);
> +
> +	if (duration < nmi_longest_ns || duration < action->max_duration)
> +		return;
> +
> +	action->max_duration = duration;
>  
>  	remainder_ns = do_div(whole_msecs, (1000 * 1000));
>  	decimal_msecs = remainder_ns / 1000;
>  
>  	printk_ratelimited(KERN_INFO
>  		"INFO: NMI handler (%ps) took too long to run: %lld.%03d msecs\n",
> -		a->handler, whole_msecs, decimal_msecs);
> +		action->handler, whole_msecs, decimal_msecs);
>  }
>  
>  static int nmi_handle(unsigned int type, struct pt_regs *regs)
> @@ -140,11 +144,7 @@ static int nmi_handle(unsigned int type, struct pt_regs *regs)
>  		delta = sched_clock() - delta;
>  		trace_nmi_handler(a->handler, (int)delta, thishandled);
>  
> -		if (delta < nmi_longest_ns || delta < a->max_duration)
> -			continue;
> -
> -		a->max_duration = delta;
> -		irq_work_queue(&a->irq_work);
> +		nmi_check_duration(a, delta);
>  	}
>  
>  	rcu_read_unlock();
> @@ -162,8 +162,6 @@ int __register_nmi_handler(unsigned int type, struct nmiaction *action)
>  	if (!action->handler)
>  		return -EINVAL;
>  
> -	init_irq_work(&action->irq_work, nmi_max_handler);
> -
>  	raw_spin_lock_irqsave(&desc->lock, flags);
>  
>  	/*

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)