linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Igor Stoppa <igor.stoppa@huawei.com>
To: Matthew Wilcox <willy@infradead.org>, Jann Horn <jannh@google.com>
Cc: jglisse@redhat.com, Kees Cook <keescook@chromium.org>,
	Michal Hocko <mhocko@kernel.org>,
	Laura Abbott <labbott@redhat.com>,
	Christoph Hellwig <hch@infradead.org>,
	Christoph Lameter <cl@linux.com>,
	linux-security-module@vger.kernel.org, linux-mm@kvack.org,
	kernel list <linux-kernel@vger.kernel.org>,
	Kernel Hardening <kernel-hardening@lists.openwall.com>
Subject: Re: [kernel-hardening] [PATCH 4/6] Protectable Memory
Date: Fri, 26 Jan 2018 13:46:55 +0200	[thread overview]
Message-ID: <4788245d-c8e1-3587-c1e0-09c18245fe9a@huawei.com> (raw)
In-Reply-To: <20180126053542.GA30189@bombadil.infradead.org>

On 26/01/18 07:35, Matthew Wilcox wrote:
> On Wed, Jan 24, 2018 at 08:10:53PM +0100, Jann Horn wrote:
>> I'm not entirely convinced by the approach of marking small parts of
>> kernel memory as readonly for hardening.
> 
> It depends how significant the data stored in there are.  For example,
> storing function pointers in read-only memory provides significant
> hardening.
> 
>> You're allocating with vmalloc(), which, as far as I know, establishes
>> a second mapping in the vmalloc area for pages that are already mapped
>> as RW through the physmap. AFAICS, later, when you're trying to make
>> pages readonly, you're only changing the protections on the second
>> mapping in the vmalloc area, therefore leaving the memory writable
>> through the physmap. Is that correct? If so, please either document
>> the reasoning why this is okay or change it.
> 
> Yes, this is still vulnerable to attacks through the physmap.  That's also
> true for marking structs as const.  We should probably fix that at some
> point, but at least they're not vulnerable to heap overruns by small
> amounts ... you have to be able to overrun some other array by terabytes.

Actually, I think there is something to say in favor of using a vmalloc
based approach, precisely because of the physmap :-P

If I understood correctly, the physmap is primarily meant to speed up
access to physical memory through the TLB. In particular, for kmalloc
based allocations.

Which means that, to perform a physmap-based attack to a kmalloced
allocation, one needs to know:

- the address of the target variable in the kmalloc range
- the randomized offset of the kernel
- the location of the physmap

But, for a vmalloc based allocation, there is one extra hoop: since the
mapping is really per page, now the attacker has actually to walk the
page table, to figure out where to poke in the physmap.


One more thought about physmap: does it map also code?
Because, if it does, and one wants to use it for an attack, isn't it
easier to look for some security test and replace a bne with be or
equivalent?


> It's worth having a discussion about whether we want the pmalloc API
> or whether we want a slab-based API.

pmalloc is meant to be useful where the attack surface is made up of
lots of small allocations - my first use case was the SE Linux policy
DB, where there is a variety of elements being allocated, in large
amount. To the point where having ready made caches would be wasteful.


Then there is the issue I already mentioned about arm/arm64 which would
require to break down large mappings, which seems to be against current
policy, as described in my previous mail:

http://www.openwall.com/lists/kernel-hardening/2018/01/24/11


I do not know exactly what you have in mind wrt slab, but my impression
is that it will most likely gravitate toward the pmalloc implementation.
It will need:

- "pools" or anyway some means to lock only a certain group of pages,
related to a specific kernel user

- (mostly) lockless allocation

- a way to manage granularity (or order of allocation)

Most of this is already provided by genalloc, which is what I ended up
almost re-implementing, before being pointed to it :-)

I only had to add the tracking of end of allocations, which is what the
patch 1/6 does - as side note, is anybody maintaining it?
I could not find an entry in MAINTAINERS

As I mentioned above, using vmalloc adds even an extra layer of protection.

The major downside is the increased TLB use, however this is not so
relevant for the volumes of data that I had to deal with so far:
only few 4K pages.

But you might have in mind something else.
I'd be interested to know what and what would be an obstacle in using
pmalloc. Maybe it can be solved.

--
igor

--
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:[~2018-01-26 11:46 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-24 17:56 [RFC PATCH v11 0/6] mm: security: ro protection for dynamic data Igor Stoppa
2018-01-24 17:56 ` [PATCH 1/6] genalloc: track beginning of allocations Igor Stoppa
2018-01-24 17:56 ` [PATCH 2/6] genalloc: selftest Igor Stoppa
2018-01-24 17:56 ` [PATCH 3/6] struct page: add field for vm_struct Igor Stoppa
2018-01-24 17:56 ` [PATCH 4/6] Protectable Memory Igor Stoppa
2018-01-24 19:10   ` [kernel-hardening] " Jann Horn
2018-01-25 11:59     ` Igor Stoppa
2018-01-25 15:14       ` Boris Lukashev
2018-01-25 15:38         ` Jerome Glisse
2018-01-26 12:28           ` Igor Stoppa
2018-01-26 16:36             ` Boris Lukashev
2018-01-30 13:46               ` Igor Stoppa
2018-01-26  5:35     ` Matthew Wilcox
2018-01-26 11:46       ` Igor Stoppa [this message]
2018-02-02 18:39       ` Christopher Lameter
2018-02-03 15:38         ` Igor Stoppa
2018-02-03 19:57           ` Igor Stoppa
2018-02-03 20:12             ` Boris Lukashev
2018-02-03 20:32               ` Igor Stoppa
2018-02-03 22:29                 ` Boris Lukashev
2018-02-04 15:05                   ` Igor Stoppa
2018-02-12 23:27                     ` Kees Cook
2018-02-13  0:40                       ` Laura Abbott
2018-02-13  1:25                         ` Kees Cook
2018-02-13  3:39                           ` Jann Horn
2018-02-13 16:09                             ` Laura Abbott
2018-02-13 21:43                               ` Kees Cook
2018-02-14 19:06                                 ` arm64 physmap (was Re: [kernel-hardening] [PATCH 4/6] Protectable Memory) Laura Abbott
2018-02-14 19:28                                   ` Ard Biesheuvel
2018-02-14 20:13                                     ` Laura Abbott
2018-02-14 19:29                                   ` Kees Cook
2018-02-14 19:35                                     ` Kees Cook
2018-02-20 16:28                                     ` Igor Stoppa
2018-02-21 22:22                                       ` Kees Cook
2018-02-14 19:48                                   ` Kees Cook
2018-02-14 22:13                                     ` Tycho Andersen
2018-02-14 22:27                                       ` Kees Cook
2018-02-13 15:20                         ` [PATCH 4/6] Protectable Memory Igor Stoppa
2018-02-13 15:20                         ` [kernel-hardening] " Igor Stoppa
     [not found]                         ` <5a83024c.64369d0a.a1e94.cdd6SMTPIN_ADDED_BROKEN@mx.google.com>
2018-02-13 18:10                           ` Laura Abbott
2018-02-20 17:16                             ` Igor Stoppa
2018-02-21 22:37                               ` Kees Cook
2018-02-05 15:40           ` Christopher Lameter
2018-02-09 11:17             ` Igor Stoppa
2018-01-26 19:41   ` Igor Stoppa
2018-01-24 17:56 ` [PATCH 5/6] Documentation for Pmalloc Igor Stoppa
2018-01-24 19:14   ` Ralph Campbell
2018-01-25  7:53     ` Igor Stoppa
2018-01-24 17:56 ` [PATCH 6/6] Pmalloc: self-test Igor Stoppa

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=4788245d-c8e1-3587-c1e0-09c18245fe9a@huawei.com \
    --to=igor.stoppa@huawei.com \
    --cc=cl@linux.com \
    --cc=hch@infradead.org \
    --cc=jannh@google.com \
    --cc=jglisse@redhat.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=labbott@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mhocko@kernel.org \
    --cc=willy@infradead.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