All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Hildenbrand <dahi@linux.vnet.ibm.com>
To: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: benh@kernel.crashing.org, paulus@samba.org,
	akpm@linux-foundation.org, heiko.carstens@de.ibm.com,
	dahi@linux.vnet.ibm.com, schwidefsky@de.ibm.com,
	borntraeger@de.ibm.com, mst@redhat.com, tglx@linutronix.de,
	David.Laight@ACULAB.COM, peterz@infradead.org, hughd@google.com,
	hocko@suse.cz
Subject: [PATCH v1 3/5] mm, uaccess: trigger might_sleep() in might_fault() when pagefaults are disabled
Date: Fri,  5 Dec 2014 12:18:07 +0100	[thread overview]
Message-ID: <1417778289-51567-4-git-send-email-dahi@linux.vnet.ibm.com> (raw)
In-Reply-To: <1417778289-51567-1-git-send-email-dahi@linux.vnet.ibm.com>

Commit 662bbcb2747c ("mm, sched: Allow uaccess in atomic with
pagefault_disable()") removed might_sleep() checks for all user access code
(that uses might_fault()).

The reason was to disable wrong "sleep in atomic" warnings in the following
scenario:
    pagefault_disable()
    rc = copy_to_user(...)
    pagefault_enable()

Which is valid, as pagefault_disable() increments the preempt counter and
therefore disables the pagefault handler. copy_to_user() will not sleep and return
an invalid return code if a page is not available.

However, as all might_sleep() checks are removed, CONFIG_DEBUG_ATOMIC_SLEEP
would no longer detect the following scenario:
    spin_lock(&lock);
    rc = copy_to_user(...)
    spin_unlock(&lock)

If the kernel is compiled with preemption turned on, the preempt counter would
be incremented and copy_to_user() would never sleep. However, with preemption
turned off, the preempt counter will not be touched, we will therefore sleep in
atomic context. We really want to enable CONFIG_DEBUG_ATOMIC_SLEEP checks for
user access functions again, otherwise horrible deadlocks might be hard to debug.

Root of all evil is that pagefault_disable() acted almost as preempt_disable(),
depending on preemption being turned on/off.

As we now have pagefault_disabled(), we can use it to distingusih whether user
acces functions might sleep.

Convert might_fault() into a makro that calls __might_fault(), to allow proper
file + line messages in case of a might_sleep() warning. We can't move the code
directly into kernel.h for now, as that results in ugly header recursions we
can't avoid for now.

Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
---
 include/linux/kernel.h |  3 ++-
 mm/memory.c            | 19 +++++++------------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 3d770f55..aa0907e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -227,7 +227,8 @@ static inline u32 reciprocal_scale(u32 val, u32 ep_ro)
 
 #if defined(CONFIG_MMU) && \
 	(defined(CONFIG_PROVE_LOCKING) || defined(CONFIG_DEBUG_ATOMIC_SLEEP))
-void might_fault(void);
+#define might_fault() __might_fault(__FILE__, __LINE__)
+void __might_fault(const char *file, int line);
 #else
 static inline void might_fault(void) { }
 #endif
diff --git a/mm/memory.c b/mm/memory.c
index d5f2ae9..54dde04 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3700,7 +3700,7 @@ void print_vma_addr(char *prefix, unsigned long ip)
 }
 
 #if defined(CONFIG_PROVE_LOCKING) || defined(CONFIG_DEBUG_ATOMIC_SLEEP)
-void might_fault(void)
+void __might_fault(const char *file, int line)
 {
 	/*
 	 * Some code (nfs/sunrpc) uses socket ops on kernel memory while
@@ -3710,21 +3710,16 @@ void might_fault(void)
 	 */
 	if (segment_eq(get_fs(), KERNEL_DS))
 		return;
-
-	/*
-	 * it would be nicer only to annotate paths which are not under
-	 * pagefault_disable, however that requires a larger audit and
-	 * providing helpers like get_user_atomic.
-	 */
-	if (in_atomic())
+	if (unlikely(!pagefault_disabled())) {
+		__might_sleep(file, line, 0);
 		return;
-
-	__might_sleep(__FILE__, __LINE__, 0);
-
+	}
+#if defined(CONFIG_DEBUG_ATOMIC_SLEEP)
 	if (current->mm)
 		might_lock_read(&current->mm->mmap_sem);
+#endif
 }
-EXPORT_SYMBOL(might_fault);
+EXPORT_SYMBOL(__might_fault);
 #endif
 
 #if defined(CONFIG_TRANSPARENT_HUGEPAGE) || defined(CONFIG_HUGETLBFS)
-- 
1.8.5.5

  parent reply	other threads:[~2014-12-05 11:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-05 11:18 [PATCH v1 0/5] Reenable might_sleep() checks for might_fault() David Hildenbrand
2014-12-05 11:18 ` [PATCH v1 1/5] uaccess: add pagefault_count to thread_info David Hildenbrand
2014-12-08 13:12   ` Christian Borntraeger
2014-12-08 13:24     ` David Hildenbrand
2014-12-05 11:18 ` [PATCH v1 2/5] uaccess: count pagefault_disable() levels in pagefault_count David Hildenbrand
2014-12-05 11:18 ` David Hildenbrand [this message]
2014-12-05 11:45   ` [PATCH v1 3/5] mm, uaccess: trigger might_sleep() in might_fault() when pagefaults are disabled Heiko Carstens
2014-12-05 11:46     ` David Hildenbrand
2014-12-05 12:08       ` David Laight
2014-12-05 12:08         ` David Laight
2014-12-05 12:08         ` David Laight
2014-12-05 13:30         ` David Hildenbrand
2014-12-05 11:18 ` [PATCH v1 4/5] uaccess: clearify that uaccess may only sleep if pagefaults are not disabled David Hildenbrand
2014-12-05 11:18 ` [PATCH v1 5/5] uaccess: CONFIG_DEBUG_PAGEFAULT_COUNT to debug pagefault_count David Hildenbrand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1417778289-51567-4-git-send-email-dahi@linux.vnet.ibm.com \
    --to=dahi@linux.vnet.ibm.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=borntraeger@de.ibm.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=hocko@suse.cz \
    --cc=hughd@google.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=schwidefsky@de.ibm.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.