mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andi Kleen <andi@firstfloor.org>
To: benh@kernel.crashing.org, haishan.bai@gmail.com,
	David.Laight@ACULAB.COM, a.p.zijlstra@chello.nl,
	darren.hart@intel.com, akpm@linux-foundation.org,
	torvalds@linux-foundation.org, ak@linux.intel.com,
	linux-kernel@vger.kernel.org, stable@kernel.org,
	tim.bird@am.sony.com
Subject: [PATCH] [2/50] mm/futex: fix futex writes on archs with SW tracking of
Date: Thu, 28 Jul 2011 16:44:06 -0700 (PDT)	[thread overview]
Message-ID: <20110728234406.56DD62403FF@tassilo.jf.intel.com> (raw)
In-Reply-To: <20110728444.299940435@firstfloor.org>

2.6.35-longterm review patch.  If anyone has any objections, please let me know.

------------------
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

[ upstream commit 2efaca927f5cd7ecd0f1554b8f9b6a9a2c329c03 ]
 dirty & young

I haven't reproduced it myself but the fail scenario is that on such
machines (notably ARM and some embedded powerpc), if you manage to hit
that futex path on a writable page whose dirty bit has gone from the PTE,
you'll livelock inside the kernel from what I can tell.

It will go in a loop of trying the atomic access, failing, trying gup to
"fix it up", getting succcess from gup, go back to the atomic access,
failing again because dirty wasn't fixed etc...

So I think you essentially hang in the kernel.

The scenario is probably rare'ish because affected architecture are
embedded and tend to not swap much (if at all) so we probably rarely hit
the case where dirty is missing or young is missing, but I think Shan has
a piece of SW that can reliably reproduce it using a shared writable
mapping & fork or something like that.

On archs who use SW tracking of dirty & young, a page without dirty is
effectively mapped read-only and a page without young unaccessible in the
PTE.

Additionally, some architectures might lazily flush the TLB when relaxing
write protection (by doing only a local flush), and expect a fault to
invalidate the stale entry if it's still present on another processor.

The futex code assumes that if the "in_atomic()" access -EFAULT's, it can
"fix it up" by causing get_user_pages() which would then be equivalent to
taking the fault.

However that isn't the case.  get_user_pages() will not call
handle_mm_fault() in the case where the PTE seems to have the right
permissions, regardless of the dirty and young state.  It will eventually
update those bits ...  in the struct page, but not in the PTE.

Additionally, it will not handle the lazy TLB flushing that can be
required by some architectures in the fault case.

Basically, gup is the wrong interface for the job.  The patch provides a
more appropriate one which boils down to just calling handle_mm_fault()
since what we are trying to do is simulate a real page fault.

The futex code currently attempts to write to user memory within a
pagefault disabled section, and if that fails, tries to fix it up using
get_user_pages().

This doesn't work on archs where the dirty and young bits are maintained
by software, since they will gate access permission in the TLB, and will
not be updated by gup().

In addition, there's an expectation on some archs that a spurious write
fault triggers a local TLB flush, and that is missing from the picture as
well.

I decided that adding those "features" to gup() would be too much for this
already too complex function, and instead added a new simpler
fixup_user_fault() which is essentially a wrapper around handle_mm_fault()
which the futex code can call.

[akpm@linux-foundation.org: coding-style fixes]
[akpm@linux-foundation.org: fix some nits Darren saw, fiddle comment layout]
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reported-by: Shan Hai <haishan.bai@gmail.com>
Tested-by: Shan Hai <haishan.bai@gmail.com>
Cc: David Laight <David.Laight@ACULAB.COM>
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Darren Hart <darren.hart@intel.com>
Cc: <stable@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>

Index: linux-2.6.35.y/include/linux/mm.h
===================================================================
--- linux-2.6.35.y.orig/include/linux/mm.h
+++ linux-2.6.35.y/include/linux/mm.h
@@ -850,6 +850,8 @@ int get_user_pages(struct task_struct *t
 int get_user_pages_fast(unsigned long start, int nr_pages, int write,
 			struct page **pages);
 struct page *get_dump_page(unsigned long addr);
+extern int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
+			    unsigned long address, unsigned int fault_flags);
 
 extern int try_to_release_page(struct page * page, gfp_t gfp_mask);
 extern void do_invalidatepage(struct page *page, unsigned long offset);
Index: linux-2.6.35.y/kernel/futex.c
===================================================================
--- linux-2.6.35.y.orig/kernel/futex.c
+++ linux-2.6.35.y/kernel/futex.c
@@ -306,8 +306,8 @@ static int fault_in_user_writeable(u32 _
 	int ret;
 
 	down_read(&mm->mmap_sem);
-	ret = get_user_pages(current, mm, (unsigned long)uaddr,
-			     1, 1, 0, NULL, NULL);
+	ret = fixup_user_fault(current, mm, (unsigned long)uaddr,
+			       FAULT_FLAG_WRITE);
 	up_read(&mm->mmap_sem);
 
 	return ret < 0 ? ret : 0;
Index: linux-2.6.35.y/mm/memory.c
===================================================================
--- linux-2.6.35.y.orig/mm/memory.c
+++ linux-2.6.35.y/mm/memory.c
@@ -1497,7 +1497,63 @@ int __get_user_pages(struct task_struct 
 	return i;
 }
 
-/**
+/*
+ * fixup_user_fault() - manually resolve a user page fault
+ * @tsk:	the task_struct to use for page fault accounting, or
+ *		NULL if faults are not to be recorded.
+ * @mm:		mm_struct of target mm
+ * @address:	user address
+ * @fault_flags:flags to pass down to handle_mm_fault()
+ *
+ * This is meant to be called in the specific scenario where for locking reasons
+ * we try to access user memory in atomic context (within a pagefault_disable()
+ * section), this returns -EFAULT, and we want to resolve the user fault before
+ * trying again.
+ *
+ * Typically this is meant to be used by the futex code.
+ *
+ * The main difference with get_user_pages() is that this function will
+ * unconditionally call handle_mm_fault() which will in turn perform all the
+ * necessary SW fixup of the dirty and young bits in the PTE, while
+ * handle_mm_fault() only guarantees to update these in the struct page.
+ *
+ * This is important for some architectures where those bits also gate the
+ * access permission to the page because they are maintained in software.  On
+ * such architectures, gup() will not be enough to make a subsequent access
+ * succeed.
+ *
+ * This should be called with the mm_sem held for read.
+ */
+int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
+		     unsigned long address, unsigned int fault_flags)
+{
+	struct vm_area_struct *vma;
+	int ret;
+
+	vma = find_extend_vma(mm, address);
+	if (!vma || address < vma->vm_start)
+		return -EFAULT;
+
+	ret = handle_mm_fault(mm, vma, address, fault_flags);
+	if (ret & VM_FAULT_ERROR) {
+		if (ret & VM_FAULT_OOM)
+			return -ENOMEM;
+		if (ret & VM_FAULT_HWPOISON)
+			return -EFAULT;
+		if (ret & VM_FAULT_SIGBUS)
+			return -EFAULT;
+		BUG();
+	}
+	if (tsk) {
+		if (ret & VM_FAULT_MAJOR)
+			tsk->maj_flt++;
+		else
+			tsk->min_flt++;
+	}
+	return 0;
+}
+
+/*
  * get_user_pages() - pin user pages in memory
  * @tsk:	task_struct of target task
  * @mm:		mm_struct of target mm

  parent reply	other threads:[~2011-07-28 23:54 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-28 23:44 [PATCH] [0/50] 2.6.35.14 longterm review Andi Kleen
2011-07-28 23:44 ` [PATCH] [1/50] x86: Make Dell Latitude E6420 use reboot=pci Andi Kleen
2011-07-28 23:44 ` Andi Kleen [this message]
2011-07-28 23:44 ` [PATCH] [3/50] mm/backing-dev.c: reset bdi min_ratio in bdi_unregister() Andi Kleen
2011-07-28 23:44 ` [PATCH] [4/50] xtensa: prevent arbitrary read in ptrace Andi Kleen
2011-07-28 23:44 ` [PATCH] [5/50] ipc/sem.c: fix race with concurrent semtimedop() timeouts Andi Kleen
2011-07-28 23:44 ` [PATCH] [6/50] jme: Fix unmap error (Causing system freeze) Andi Kleen
2011-07-28 23:44 ` [PATCH] [7/50] fix crash in scsi_dispatch_cmd() Andi Kleen
2011-07-28 23:44 ` [PATCH] [8/50] mmc: Add PCI fixup quirks for Ricoh 1180:e823 reader Andi Kleen
2011-07-28 23:44 ` [PATCH] [9/50] mmc: Added quirks for Ricoh 1180:e823 lower base clock Andi Kleen
2011-07-29  0:27   ` matsumur
2011-07-29  0:37     ` Andi Kleen
2011-07-29  0:46       ` matsumur
2011-07-29  0:47       ` Chris Ball
2011-07-29  2:10         ` matsumur
2011-07-29  2:49           ` Chris Ball
2011-07-28 23:44 ` [PATCH] [10/50] Drop -Werror in perf Andi Kleen
2011-07-28 23:44 ` [PATCH] [11/50] kexec, x86: Fix incorrect jump back address if not Andi Kleen
2011-07-28 23:44 ` [PATCH] [12/50] USB: serial: add IDs for WinChipHead USB->RS232 adapter Andi Kleen
2011-07-28 23:44 ` [PATCH] [13/50] davinci: DM365 EVM: fix video input mux bits Andi Kleen
2011-07-28 23:44 ` [PATCH] [14/50] powerpc/pseries/hvconsole: Fix dropped console output Andi Kleen
2011-07-28 23:44 ` [PATCH] [15/50] hvc_console: Improve tty/console put_chars handling Andi Kleen
2011-07-28 23:44 ` [PATCH] [16/50] powerpc/kdump: Fix timeout in crash_kexec_wait_realmode Andi Kleen
2011-07-28 23:44 ` [PATCH] [17/50] si4713-i2c: avoid potential buffer overflow on si4713 Andi Kleen
2011-07-28 23:44 ` [PATCH] [18/50] hwmon: (max1111) Fix race condition causing NULL pointer Andi Kleen
2011-07-28 23:44 ` [PATCH] [19/50] hwmon: (asus_atk0110) Fix memory leak Andi Kleen
2011-07-28 23:44 ` [PATCH] [20/50] USB: OHCI: fix another regression for NVIDIA controllers Andi Kleen
2011-07-28 23:44 ` [PATCH] [21/50] firewire: cdev: prevent race between first get_info ioctl Andi Kleen
2011-07-28 23:44 ` [PATCH] [22/50] firewire: cdev: return -ENOTTY for unimplemented ioctls, not Andi Kleen
2011-07-28 23:44 ` [PATCH] [23/50] svcrpc: fix list-corrupting race on nfsd shutdown Andi Kleen
2011-07-28 23:44 ` [PATCH] [24/50] x86: Look for IA32_ENERGY_PERF_BIAS support Andi Kleen
2011-07-28 23:44 ` [PATCH] [25/50] x86, intel, power: Initialize MSR_IA32_ENERGY_PERF_BIAS Andi Kleen
2011-07-28 23:44 ` [PATCH] [26/50] SUNRPC: Fix use of static variable in rpcb_getport_async Andi Kleen
2011-07-28 23:44 ` [PATCH] [27/50] ARM: pxa/cm-x300: fix V3020 RTC functionality Andi Kleen
2011-07-28 23:44 ` [PATCH] [28/50] firewire: ohci: do not bind to Pinnacle cards, avert panic Andi Kleen
2011-07-28 23:44 ` [PATCH] [29/50] mm/nommu.c: fix remap_pfn_range() Andi Kleen
2011-07-28 23:44 ` [PATCH] [30/50] EHCI: only power off port if over-current is active Andi Kleen
2011-07-28 23:44 ` [PATCH] [31/50] mac80211: Restart STA timers only on associated state Andi Kleen
2011-07-28 23:44 ` [PATCH] [32/50] usb: musb: restore INDEX register in resume path Andi Kleen
2011-07-28 23:44 ` [PATCH] [33/50] SUNRPC: Fix a race between work-queue and rpc_killall_tasks Andi Kleen
2011-07-28 23:44 ` [PATCH] [34/50] bttv: fix s_tuner for radio Andi Kleen
2011-07-28 23:44 ` [PATCH] [35/50] pvrusb2: fix g/s_tuner support Andi Kleen
2011-07-28 23:44 ` [PATCH] [36/50] v4l2-ioctl.c: prefill tuner type for g_frequency and Andi Kleen
2011-07-28 23:44 ` [PATCH] [37/50] mac80211: fix TKIP replay vulnerability Andi Kleen
2011-07-28 23:44 ` [PATCH] [38/50] ASoC: ak4642: fixup snd_soc_update_bits mask for PW_MGMT2 Andi Kleen
2011-07-28 23:44 ` [PATCH] [39/50] tracing: Fix bug when reading system filters on module Andi Kleen
2011-07-28 23:44 ` [PATCH] [40/50] tracing: Have "enable" file use refcounts like the "filter" Andi Kleen
2011-07-28 23:44 ` [PATCH] [41/50] ARM: pxa: fix PGSR register address calculation Andi Kleen
2011-07-28 23:44 ` [PATCH] [42/50] iommu/amd: Don't use MSI address range for DMA addresses Andi Kleen
2011-07-28 23:44 ` [PATCH] [43/50] staging: r8192e_pci: Handle duplicate PCI ID 0x10ec:0x8192 Andi Kleen
2011-07-28 23:44 ` [PATCH] [44/50] staging: comedi: fix infoleak to userspace Andi Kleen
2011-07-28 23:44 ` [PATCH] [45/50] Staging: hv: netvsc: Fix a bug in accounting transmit slots Andi Kleen
2011-07-28 23:44 ` [PATCH] [46/50] ARM: 6989/1: perf: do not start the PMU when no events are Andi Kleen
2011-07-28 23:44 ` [PATCH] [47/50] ASoC: Ensure we delay long enough for WM8994 FLL to lock Andi Kleen
2011-07-28 23:44 ` [PATCH] [48/50] SERIAL: SC26xx: Fix link error Andi Kleen
2011-07-28 23:44 ` [PATCH] [49/50] x86, mtrr: lock stop machine during MTRR rendezvous sequence Andi Kleen
2011-07-28 23:44 ` [PATCH] [50/50] ipv6: add special mode forwarding=2 to send RS while Andi Kleen

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=20110728234406.56DD62403FF@tassilo.jf.intel.com \
    --to=andi@firstfloor.org \
    --cc=David.Laight@ACULAB.COM \
    --cc=a.p.zijlstra@chello.nl \
    --cc=ak@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=darren.hart@intel.com \
    --cc=haishan.bai@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@kernel.org \
    --cc=tim.bird@am.sony.com \
    --cc=torvalds@linux-foundation.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®