linux-kernel - Re: [PATCH v10 03/19] arm: fiq: Replace default FIQ 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]

Date:	Thu, 28 Aug 2014 08:43:09 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	Daniel Thompson <daniel.thompson@...aro.org>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	kgdb-bugreport@...ts.sourceforge.net, patches@...aro.org,
	linaro-kernel@...ts.linaro.org,
	John Stultz <john.stultz@...aro.org>,
	Anton Vorontsov <anton.vorontsov@...aro.org>,
	Colin Cross <ccross@...roid.com>, kernel-team@...roid.com,
	Rob Herring <robherring2@...il.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	Ben Dooks <ben.dooks@...ethink.co.uk>,
	Catalin Marinas <catalin.marinas@....com>,
	Dave Martin <Dave.Martin@....com>,
	Fabio Estevam <festevam@...il.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Nicolas Pitre <nico@...aro.org>
Subject: Re: [PATCH v10 03/19] arm: fiq: Replace default FIQ handler

On Thu, Aug 28, 2014 at 04:01:12PM +0100, Russell King - ARM Linux wrote:
> On Tue, Aug 19, 2014 at 07:12:07PM +0100, Daniel Thompson wrote:
> > On 19/08/14 18:37, Russell King - ARM Linux wrote:
> > > On Tue, Aug 19, 2014 at 05:45:53PM +0100, Daniel Thompson wrote:
> > >> +int register_fiq_nmi_notifier(struct notifier_block *nb)
> > >> +{
> > >> +	return atomic_notifier_chain_register(&fiq_nmi_chain, nb);
> > >> +}
> > >> +
> > >> +asmlinkage void __exception_irq_entry fiq_nmi_handler(struct pt_regs *regs)
> > >> +{
> > >> +	struct pt_regs *old_regs = set_irq_regs(regs);
> > >> +
> > >> +	nmi_enter();
> > >> +	atomic_notifier_call_chain(&fiq_nmi_chain, (unsigned long)regs, NULL);
> > >> +	nmi_exit();
> > >> +	set_irq_regs(old_regs);
> > >> +}
> > > 
> > > Really not happy with this.  What happens if a FIQ occurs while we're
> > > inside register_fiq_nmi_notifier() - more specifically inside
> > > atomic_notifier_chain_register() ?
> > 
> > Should depend on which side of the rcu update we're on.
> 
> I just asked Paul McKenney, our RCU expert... essentially, yes, RCU
> stuff itself is safe in this context.  However, RCU stuff can call into
> lockdep if lockdep is configured, and there are questions over lockdep.
> 
> There's some things which can be done to reduce the lockdep exposure
> to it, such as ensuring that rcu_read_lock() is first called outside
> of FIQ context.
> 
> There's concerns with whether either printk() in check_flags() could
> be reached too (flags there should always indicate that IRQs were
> disabled, so that reduces down to a question about just the first
> printk() there.)
> 
> There's also the very_verbose() stuff for RCU lockdep classes which
> Paul says must not be enabled.
> 
> Lastly, Paul isn't a lockdep expert, but he sees nothing that prevents
> lockdep doing the deadlock checking as a result of the above call.
> 
> So... this coupled with my feeling that notifiers make it too easy for
> unreviewed code to be hooked into this path, I'm fairly sure that we
> don't want to be calling atomic notifier chains from FIQ context.

In the worst case, it would be possible to create a parallel notifier
that was intended for use from NMI.  There would be no need for
rcu_read_lock() in that case, we would instead be using RCU-sched,
for which NMI handlers are automatically RCU-sched read-side critical
sections.  Instead of synchronize_rcu(), this NMI version would use
synchronize_sched().

But if lockdep works from NMI, then the current notifiers would work
just fine.

							Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/