Re: 3.15rc2 hanging processes on exit.

[Hugh Dickins]

On Tue, 22 Apr 2014, Linus Torvalds wrote:
> On Tue, Apr 22, 2014 at 11:03 AM, Dave Jones <davej@xxxxxxxxxx> wrote:
> > I've got a test box that's running my fuzzer that is in an odd state.
> > The processes are about to end, but they don't seem to be making any
> > progress.  They've been spinning in the same state for a few hours now..
> >
> > perf top -a is showing a lot of time is being spent in page_fault and bad_gs
> >
> > there's a large trace file here from the function tracer:
> > http://codemonkey.org.uk/junk/trace.out
> 
> The trace says that it's one of the infinite loops that do
> 
>  - cmpxchg_futex_value_locked() fails
>  - we do fault_in_user_writeable(FAULT_FLAG_WRITE) and that succeeds
>  - so we try again
> 
> So it implies that handle_mm_fault() returned without VM_FAULT_ERROR,
> but the page still isn't actually writable.
> 
> And to me that smells like (vm_flags & VM_WRITE) isn't set. We'll
> fault in the page all right, but the resulting page table entry still
> isn't writable.
> 
> Are you testing anything new? Or is this strictly new to 3.15? The
> only thing in this area we do differently is commit cda540ace6a1 ("mm:
> get_user_pages(write,force) refuse to COW in shared areas"), but
> fault_in_user_writeable() never used the force bit afaik. Adding Hugh
> just in case.
> 
> So I think we should make fault_in_user_writeable() just check the
> vm_flags. Something like the attached (UNTESTED!) patch.
> 
> Guys? Comments?

Your patch looks to me correct and to the point; but I agree that
we haven't made a relevant change there recently, so I suppose it
comes from a trinity improvement rather than a new bug in 3.15.

(Dave, do you have time to confirm that by running new trinity on 3.14?)

One nit: we're inconsistent, and shall never move VM_READ,VM_WRITE bits,
but it would set a better example to declare "vm_flags_t vm_flags"
in your patch below, instead of "unsigned vm_flags".

Hugh
---

 mm/memory.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index d0f0bef3be48..91a3e848745d 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1955,12 +1955,17 @@ int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
 		     unsigned long address, unsigned int fault_flags)
 {
 	struct vm_area_struct *vma;
+	unsigned vm_flags;
 	int ret;
 
 	vma = find_extend_vma(mm, address);
 	if (!vma || address < vma->vm_start)
 		return -EFAULT;
 
+	vm_flags = (fault_flags & FAULT_FLAG_WRITE) ? VM_WRITE : VM_READ;
+	if (!(vm_flags & vma->vm_flags))
+		return -EFAULT;
+
 	ret = handle_mm_fault(mm, vma, address, fault_flags);
 	if (ret & VM_FAULT_ERROR) {
 		if (ret & VM_FAULT_OOM)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/