Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1163078AbbKFUtX (ORCPT ); Fri, 6 Nov 2015 15:49:23 -0500 Received: from mail-ig0-f181.google.com ([209.85.213.181]:37353 "EHLO mail-ig0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161784AbbKFUtV (ORCPT ); Fri, 6 Nov 2015 15:49:21 -0500 MIME-Version: 1.0 In-Reply-To: <20151106192205.899033919@linuxfoundation.org> References: <20151106192205.351595349@linuxfoundation.org> <20151106192205.899033919@linuxfoundation.org> Date: Fri, 6 Nov 2015 12:49:20 -0800 X-Google-Sender-Auth: DQEVOOiBNoFNjZXDeHe156wA6Qc Message-ID: Subject: Re: [PATCH 4.1 11/86] iommu/amd: Fix BUG when faulting a PROT_NONE VMA From: Linus Torvalds To: Greg Kroah-Hartman Cc: Linux Kernel Mailing List , stable , Jay Cornwall , Joerg Roedel Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1581 Lines: 47 On Fri, Nov 6, 2015 at 11:22 AM, Greg Kroah-Hartman wrote: > > From: Jay Cornwall > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/