All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: akpm@linux-foundation.org, luto@kernel.org, will.deacon@arm.com,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	kernel-hardening@lists.openwall.com,
	naveen.n.rao@linux.vnet.ibm.com, anil.s.keshavamurthy@intel.com,
	davem@davemloft.net, rostedt@goodmis.org, mingo@redhat.com,
	ast@kernel.org, daniel@iogearbox.net, jeyu@kernel.org,
	netdev@vger.kernel.org, ard.biesheuvel@linaro.org,
	jannh@google.com, kristen@linux.intel.com, dave.hansen@intel.com,
	deneen.t.dock@intel.com
Subject: Re: [PATCH 0/2] Don’t leave executable TLB entries to freed pages
Date: Thu, 29 Nov 2018 23:06:16 +0900	[thread overview]
Message-ID: <20181129230616.f017059a093841dbaa4b82e6@kernel.org> (raw)
In-Reply-To: <20181128000754.18056-1-rick.p.edgecombe@intel.com>

On Tue, 27 Nov 2018 16:07:52 -0800
Rick Edgecombe <rick.p.edgecombe@intel.com> wrote:

> Sometimes when memory is freed via the module subsystem, an executable
> permissioned TLB entry can remain to a freed page. If the page is re-used to
> back an address that will receive data from userspace, it can result in user
> data being mapped as executable in the kernel. The root of this behavior is
> vfree lazily flushing the TLB, but not lazily freeing the underlying pages. 

Good catch!

> 
> There are sort of three categories of this which show up across modules, bpf,
> kprobes and ftrace:

For x86-64 kprobe, it sets the page NX and after that RW, and then release
via module_memfree. So I'm not sure it really happens on kprobes. (Of course
the default memory allocator is simpler so it may happen on other archs) But
interesting fixes.

Thank you,


> 
> 1. When executable memory is touched and then immediatly freed
> 
>    This shows up in a couple error conditions in the module loader and BPF JIT
>    compiler.
> 
> 2. When executable memory is set to RW right before being freed
> 
>    In this case (on x86 and probably others) there will be a TLB flush when its
>    set to RW and so since the pages are not touched between setting the
>    flush and the free, it should not be in the TLB in most cases. So this
>    category is not as big of a concern. However, techinically there is still a
>    race where an attacker could try to keep it alive for a short window with a
>    well timed out-of-bound read or speculative read, so ideally this could be
>    blocked as well.
> 
> 3. When executable memory is freed in an interrupt
> 
>    At least one example of this is the freeing of init sections in the module
>    loader. Since vmalloc reuses the allocation for the work queue linked list
>    node for the deferred frees, the memory actually gets touched as part of the
>    vfree operation and so returns to the TLB even after the flush from resetting
>    the permissions.
> 
> I have only actually tested category 1, and identified 2 and 3 just from reading
> the code.
> 
> To catch all of these, module_alloc for x86 is changed to use a new flag that
> instructs the unmap operation to flush the TLB before freeing the pages.
> 
> If this solution seems good I can plug the flag in for other architectures that
> define PAGE_KERNEL_EXEC.
> 
> 
> Rick Edgecombe (2):
>   vmalloc: New flag for flush before releasing pages
>   x86/modules: Make x86 allocs to flush when free
> 
>  arch/x86/kernel/module.c |  4 ++--
>  include/linux/vmalloc.h  |  1 +
>  mm/vmalloc.c             | 13 +++++++++++--
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> -- 
> 2.17.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

  parent reply	other threads:[~2018-11-29 14:06 UTC|newest]

Thread overview: 117+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-28  0:07 [PATCH 0/2] Don’t leave executable TLB entries to freed pages Rick Edgecombe
2018-11-28  0:07 ` [PATCH 1/2] vmalloc: New flag for flush before releasing pages Rick Edgecombe
2018-12-04  0:04   ` Edgecombe, Rick P
2018-12-04  0:04     ` Edgecombe, Rick P
2018-12-04  0:04     ` Edgecombe, Rick P
2018-12-04  0:04     ` Edgecombe, Rick P
2018-12-04  1:43   ` Nadav Amit
2018-12-04 16:03     ` Will Deacon
2018-12-04 20:02       ` Edgecombe, Rick P
2018-12-04 20:02         ` Edgecombe, Rick P
2018-12-04 20:02         ` Edgecombe, Rick P
2018-12-04 20:02         ` Edgecombe, Rick P
2018-12-04 20:09         ` Andy Lutomirski
2018-12-04 20:09           ` Andy Lutomirski
2018-12-04 23:52           ` Edgecombe, Rick P
2018-12-04 23:52             ` Edgecombe, Rick P
2018-12-04 23:52             ` Edgecombe, Rick P
2018-12-05  1:57             ` Andy Lutomirski
2018-12-05  1:57               ` Andy Lutomirski
2018-12-05  1:57               ` Andy Lutomirski
2018-12-05 11:41           ` Will Deacon
2018-12-05 11:41             ` Will Deacon
2018-12-05 23:16             ` Andy Lutomirski
2018-12-05 23:16               ` Andy Lutomirski
2018-12-06  7:29               ` Ard Biesheuvel
2018-12-06  7:29                 ` Ard Biesheuvel
2018-12-06 11:10                 ` Will Deacon
2018-12-06 11:10                   ` Will Deacon
2018-12-06 18:53                 ` Andy Lutomirski
2018-12-06 18:53                   ` Andy Lutomirski
2018-12-06 19:01                   ` Tycho Andersen
2018-12-06 19:01                     ` Tycho Andersen
2018-12-06 19:19                     ` Andy Lutomirski
2018-12-06 19:19                       ` Andy Lutomirski
2018-12-06 19:39                       ` Nadav Amit
2018-12-06 19:39                         ` Nadav Amit
2018-12-06 20:17                         ` Andy Lutomirski
2018-12-06 20:17                           ` Andy Lutomirski
2018-12-06 23:08                           ` Nadav Amit
2018-12-06 23:08                             ` Nadav Amit
2018-12-07  3:06                             ` Edgecombe, Rick P
2018-12-07  3:06                               ` Edgecombe, Rick P
2018-12-07  3:06                               ` Edgecombe, Rick P
2018-12-06 20:19                       ` Edgecombe, Rick P
2018-12-06 20:19                         ` Edgecombe, Rick P
2018-12-06 20:19                         ` Edgecombe, Rick P
2018-12-06 20:26                         ` Andy Lutomirski
2018-12-06 20:26                           ` Andy Lutomirski
2018-12-06 19:04                   ` Ard Biesheuvel
2018-12-06 19:04                     ` Ard Biesheuvel
2018-12-06 19:20                     ` Andy Lutomirski
2018-12-06 19:20                       ` Andy Lutomirski
2018-12-06 19:23                       ` Ard Biesheuvel
2018-12-06 19:23                         ` Ard Biesheuvel
2018-12-06 19:31                         ` Will Deacon
2018-12-06 19:31                           ` Will Deacon
2018-12-06 19:36                           ` Ard Biesheuvel
2018-12-06 19:36                             ` Ard Biesheuvel
2018-12-04 20:36         ` Nadav Amit
2018-12-04 20:36           ` Nadav Amit
2018-12-04 20:36           ` Nadav Amit
2018-12-04 23:51           ` Edgecombe, Rick P
2018-12-04 23:51             ` Edgecombe, Rick P
2018-12-05  0:01             ` Nadav Amit
2018-12-05  0:01               ` Nadav Amit
2018-12-05  0:01               ` Nadav Amit
2018-12-05  0:29               ` Edgecombe, Rick P
2018-12-05  0:29                 ` Edgecombe, Rick P
2018-12-05  0:29                 ` Edgecombe, Rick P
2018-12-05  0:53                 ` Nadav Amit
2018-12-05  0:53                   ` Nadav Amit
2018-12-05  0:53                   ` Nadav Amit
2018-12-05  1:45                   ` Edgecombe, Rick P
2018-12-05  1:45                     ` Edgecombe, Rick P
2018-12-05  1:45                     ` Edgecombe, Rick P
2018-12-05  2:09                     ` Nadav Amit
2018-12-05  2:09                       ` Nadav Amit
2018-12-05  2:09                       ` Nadav Amit
2018-12-04 18:56     ` Andy Lutomirski
2018-12-04 18:56       ` Andy Lutomirski
2018-12-04 19:44       ` Nadav Amit
2018-12-04 19:44         ` Nadav Amit
2018-12-04 19:48         ` Andy Lutomirski
2018-12-04 19:48           ` Andy Lutomirski
2018-12-04 22:48           ` Nadav Amit
2018-12-04 22:48             ` Nadav Amit
2018-12-04 23:27             ` Andy Lutomirski
2018-12-04 23:27               ` Andy Lutomirski
2018-12-04 23:34               ` Nadav Amit
2018-12-04 23:34                 ` Nadav Amit
2018-12-05  1:09             ` Edgecombe, Rick P
2018-12-05  1:09               ` Edgecombe, Rick P
2018-12-05  1:09               ` Edgecombe, Rick P
2018-12-05  1:45               ` Nadav Amit
2018-12-05  1:45                 ` Nadav Amit
2018-12-05  1:45                 ` Nadav Amit
2018-11-28  0:07 ` [PATCH 2/2] x86/modules: Make x86 allocs to flush when free Rick Edgecombe
2018-11-28 23:11   ` Andrew Morton
2018-11-29  0:02     ` Edgecombe, Rick P
2018-11-29  0:02       ` Edgecombe, Rick P
2018-11-29  0:02       ` Edgecombe, Rick P
2018-11-29  1:40   ` Andy Lutomirski
2018-11-29  1:40     ` Andy Lutomirski
2018-11-29  6:14     ` Edgecombe, Rick P
2018-11-29  6:14       ` Edgecombe, Rick P
2018-11-29  6:14       ` Edgecombe, Rick P
2018-11-28  1:06 ` [PATCH 0/2] Don’t leave executable TLB entries to freed pages Nadav Amit
2018-11-28  1:21   ` Nadav Amit
2018-11-28  9:57     ` Will Deacon
2018-11-28 18:29       ` Nadav Amit
2018-11-29 14:06 ` Masami Hiramatsu [this message]
2018-11-29 18:49   ` Edgecombe, Rick P
2018-11-29 18:49     ` Edgecombe, Rick P
2018-11-29 18:49     ` Edgecombe, Rick P
2018-11-29 23:19     ` Masami Hiramatsu
2018-11-29 23:19       ` Masami Hiramatsu
2018-11-29 23:19       ` Masami Hiramatsu

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=20181129230616.f017059a093841dbaa4b82e6@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=anil.s.keshavamurthy@intel.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=dave.hansen@intel.com \
    --cc=davem@davemloft.net \
    --cc=deneen.t.dock@intel.com \
    --cc=jannh@google.com \
    --cc=jeyu@kernel.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=kristen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=naveen.n.rao@linux.vnet.ibm.com \
    --cc=netdev@vger.kernel.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=rostedt@goodmis.org \
    --cc=will.deacon@arm.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.