linux-next.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Dave Hansen <dave@sr71.net>
Cc: linux-kernel@vger.kernel.org, dave.hansen@linux.intel.com,
	avagin@gmail.com, linux-next@vger.kernel.org, linux-mm@kvack.org,
	x86@kernel.org
Subject: Re: [PATCH] x86, pkeys: fix access_error() denial of writes to write-only VMA
Date: Wed, 2 Mar 2016 00:44:02 +0300	[thread overview]
Message-ID: <20160301214402.GA20162@node.shutemov.name> (raw)
In-Reply-To: <20160301194133.65D0110C@viggo.jf.intel.com>

On Tue, Mar 01, 2016 at 11:41:33AM -0800, Dave Hansen wrote:
> 
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> Andrey Wagin reported that a simple test case was broken by:
> 
> 	2b5f7d013fc ("mm/core, x86/mm/pkeys: Add execute-only protection keys support")
> 
> This test case creates an unreadable VMA and my patch assumed
> that all writes must be to readable VMAs.
> 
> The simplest fix for this is to remove the pkey-related bits
> in access_error().  For execute-only support, I believe the
> existing version is sufficient because the permissions we
> are trying to enforce are entirely expressed in vma->vm_flags.
> We just depend on pkeys to get *an* exception, it does not
> matter that PF_PK was set, or even what state PKRU is in.
> 
> I will re-add the necessary bits with the full pkeys
> implementation that includes the new syscalls.
> 
> The three cases that matter are:
> 
> 1. If a write to an execute-only VMA occurs, we will see PF_WRITE
>    set, but !VM_WRITE on the VMA, and return 1.  All execute-only
>    VMAs have VM_WRITE clear by definition.
> 2. If a read occurs on a present PTE, we will fall in to the "read,
>    present" case and return 1.
> 3. If a read occurs to a non-present PTE, we will miss the "read,
>    not present" case, because the execute-only VMA will have
>    VM_EXEC set, and we will properly return 0 allowing the PTE to
>    be populated.
> 
> Test program:
> 
> #include <sys/mman.h>
> #include <stdlib.h>
> 
> int main()
> {
> 	int *p;
> 	p = mmap(NULL, 4096, PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> 	p[0] = 1;
> 
> 	return 0;
> }
> 
> Fixes: 62b5f7d013fc ("mm/core, x86/mm/pkeys: Add execute-only protection keys support")
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
> Cc: Andrey Wagin <avagin@gmail.com>,
> Cc: linux-next@vger.kernel.org
> Cc: linux-mm@kvack.org
> Cc: x86@kernel.org

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

      reply	other threads:[~2016-03-01 21:44 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-01 19:41 [PATCH] x86, pkeys: fix access_error() denial of writes to write-only VMA Dave Hansen
2016-03-01 21:44 ` Kirill A. Shutemov [this message]

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=20160301214402.GA20162@node.shutemov.name \
    --to=kirill@shutemov.name \
    --cc=avagin@gmail.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=dave@sr71.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-next@vger.kernel.org \
    --cc=x86@kernel.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;
as well as URLs for NNTP newsgroup(s).