Messages in this thread | | | Date | Fri, 6 Nov 2015 12:49:20 -0800 | Subject | Re: [PATCH 4.1 11/86] iommu/amd: Fix BUG when faulting a PROT_NONE VMA | From | Linus Torvalds <> |
| |
On Fri, Nov 6, 2015 at 11:22 AM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > From: Jay Cornwall <jay@jcornwall.me> > > commit d14f6fced5f9360edca5a1325ddb7077aab1203b upstream. > > handle_mm_fault indirectly triggers a BUG in do_numa_page > when given a VMA without read/write/execute access. Check > this condition in do_fault.
This reminds me.
I think the code is still wrong.
The thing is, the VM assumes that the caller has already checked permissions. An dby "checked permissions", I mean actually checking permissions. The AMD iommu driver doesn't do that, it does something completely different, namely "check it's not PROT_NONE".
So I think the code should instead do something like
if ((write && !(vma->vm_flags & VM_WRITE)) || !(vma->vm_flags & VM_READ)) { up_read(&mm->mmap_sem); handle_fault_error(fault); goto out; }
because it is *not* valid to call "handle_mm_fault()" with a write fault unless you have write permissions (or with a read fault unless you have read permissions).
And some "handle_mm_fault would BUG_ON()" comment is just bogus. It's not handle_mm_fault()'s case that you called it without checking proper permissions.
I'm not arguing against the stable backport, because that is fine. But I think this should be fixed further.
Joerg?
Linus
| |