linux-kernel - Re: [kernel-hardening] [PATCH 4/6] Protectable Memory

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180126053542.GA30189@bombadil.infradead.org>
Date:   Thu, 25 Jan 2018 21:35:42 -0800
From:   Matthew Wilcox <willy@...radead.org>
To:     Jann Horn <jannh@...gle.com>
Cc:     Igor Stoppa <igor.stoppa@...wei.com>, jglisse@...hat.com,
        Kees Cook <keescook@...omium.org>,
        Michal Hocko <mhocko@...nel.org>,
        Laura Abbott <labbott@...hat.com>,
        Christoph Hellwig <hch@...radead.org>,
        Christoph Lameter <cl@...ux.com>,
        linux-security-module@...r.kernel.org, linux-mm@...ck.org,
        kernel list <linux-kernel@...r.kernel.org>,
        Kernel Hardening <kernel-hardening@...ts.openwall.com>
Subject: Re: [kernel-hardening] [PATCH 4/6] Protectable Memory

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.

It's worth having a discussion about whether we want the pmalloc API
or whether we want a slab-based API.  We can have a separate discussion
about an API to remove pages from the physmap.