linux-kernel - Re: [PATCH RFC 01/15] uaccess: count pagefault_disable() levels in pagefault

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: <20150507125053.5d2e8f0a@thinkpad-w530>
Date:	Thu, 7 May 2015 12:50:53 +0200
From:	David Hildenbrand <dahi@...ux.vnet.ibm.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	linux-kernel@...r.kernel.org, mingo@...hat.com,
	yang.shi@...driver.com, bigeasy@...utronix.de,
	benh@...nel.crashing.org, paulus@...ba.org,
	akpm@...ux-foundation.org, heiko.carstens@...ibm.com,
	schwidefsky@...ibm.com, borntraeger@...ibm.com, mst@...hat.com,
	tglx@...utronix.de, David.Laight@...LAB.COM, hughd@...gle.com,
	hocko@...e.cz, ralf@...ux-mips.org, herbert@...dor.apana.org.au,
	linux@....linux.org.uk, airlied@...ux.ie, daniel.vetter@...el.com,
	linux-mm@...ck.org, linux-arch@...r.kernel.org, tglx@...utronix.de
Subject: Re: [PATCH RFC 01/15] uaccess: count pagefault_disable() levels in
 pagefault_disabled

> On Wed, May 06, 2015 at 07:50:25PM +0200, David Hildenbrand wrote:
> > +/*
> > + * Is the pagefault handler disabled? If so, user access methods will not sleep.
> > + */
> > +#define pagefault_disabled() (current->pagefault_disabled != 0)
> 
> So -RT has:
> 
> static inline bool pagefault_disabled(void)
> {
> 	return current->pagefault_disabled || in_atomic();
> }
> 
> AFAICR we did this to avoid having to do both:
> 
> 	preempt_disable();
> 	pagefault_disable();
> 
> in a fair number of places -- just like this patch-set does, this is
> touching two cachelines where one would have been enough.
> 
> Also, removing in_atomic() from fault handlers like you did
> significantly changes semantics for interrupts (soft, hard and NMI).
> 
> So while I agree with most of these patches, I'm very hesitant on the
> above little detail.
> 

Just to make sure we have a common understanding (as written in my cover
letter):

Your suggestion won't work with !CONFIG_PREEMPT (!CONFIG_PREEMPT_COUNT). If
there is no preempt counter, in_atomic() won't work. So doing a
preempt_disable() instead of a pagefault_disable() is not going to work.
(not sure how -RT handles that - most probably with CONFIG_PREEMPT_COUNT being
enabled, due to atomic debug).

That's why I dropped that check for a reason.

So in my opinion, in_atomic() should never be used in any fault handler - it
has nothing to do with disabled pagefaults. It doesn't give us anything more
besides some false security for atomic environments.

This patchset is about decoupling both concept. (not ending up with to
mechanisms doing almost the same)

That's also what Thomas Gleixner suggested
https://lkml.org/lkml/2014/11/27/820 .

David

--
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/