From patchwork Tue Sep 12 18:13:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tycho Andersen X-Patchwork-Id: 9949891 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 21B936024A for ; Tue, 12 Sep 2017 18:13:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0F5C029088 for ; Tue, 12 Sep 2017 18:13:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 03EF52908A; Tue, 12 Sep 2017 18:13:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id 66FEA29089 for ; Tue, 12 Sep 2017 18:13:20 +0000 (UTC) Received: (qmail 20130 invoked by uid 550); 12 Sep 2017 18:13:19 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 20088 invoked from network); 12 Sep 2017 18:13:18 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=docker.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=X1BFGC/4V7wA0bnRs6NZmFAiob7urPaKyaViZtXLRbs=; b=G0zw+pR5Ri6YYI5HhvySLLwBy26ILLlXJ79mgvL3v6tvbmyVU2XGBjCCCbZL/qXwmc syE8OzmU1zmsHlTacd3rKpiMMmlpfk2l1FhuARD8oHSlqzBxbfx4uteNcEZdF0KLFjEs q7Ui+Et5pxWKU/Y8CoZcnxMxBdhlvpIqMoutk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=X1BFGC/4V7wA0bnRs6NZmFAiob7urPaKyaViZtXLRbs=; b=H9H74f12rGqwWCZWVXenD3JlWCIaB51SI/XAZsKj1FSCEH1Y3LbuKatPCJs1edMXnO lC/9YHaNquthlW/SBQ/F6tKgK1RMfQ1Vf+gau9KnD3sJzAlKNcH9M1dFI32sPjcaZSI8 YvS75OrMwrGONXtdjl7SoNKdW9e7Omf5pK1XKyLfbXJ8a1d+4aby6HQCuhIQ8zs+WO2q qlQa+Tbdw2P1ki25TlvN3bheNG8jpS7lQB7gOvGH5L8RJURUek2TR9LDUCPxPEkhS3iA gz95s9YdIuCn3AaH9cwUfViAEusx697p+xGIyRu3NjucBCn/HVtaU3Ldlx+Z3Cm4WsIe DO7w== X-Gm-Message-State: AHPjjUg94GYel1i9FzffwlgrcWjv8GNDU6Je+6AsmlNHFaHKg5R7EhH4 Kon2DrstOA/62zPc X-Google-Smtp-Source: AOwi7QDI6DxXXx6SfdJm50/kw7xjUGSROfl0L7P8l2eocH5w76j302y1owWTi36ngjNOVIZwfsqtmQ== X-Received: by 10.99.160.2 with SMTP id r2mr2236883pge.158.1505239986003; Tue, 12 Sep 2017 11:13:06 -0700 (PDT) Date: Tue, 12 Sep 2017 11:13:03 -0700 From: Tycho Andersen To: Yisheng Xie Cc: Juerg Haefliger , linux-kernel@vger.kernel.org, linux-mm@kvack.org, kernel-hardening@lists.openwall.com, Marco Benatto , x86@kernel.org Message-ID: <20170912181303.aqjj5ri3mhscw63t@docker> References: <20170907173609.22696-1-tycho@docker.com> <20170907173609.22696-4-tycho@docker.com> <302be94d-7e44-001d-286c-2b0cd6098f7b@huawei.com> <20170911145020.fat456njvyagcomu@docker> <57e95ad2-81d8-bf83-3e78-1313daa1bb80@canonical.com> <431e2567-7600-3186-1489-93b855c395bd@huawei.com> <20170912143636.avc3ponnervs43kj@docker> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170912143636.avc3ponnervs43kj@docker> User-Agent: NeoMutt/20170113 (1.7.2) Subject: [kernel-hardening] Re: [PATCH v6 03/11] mm, x86: Add support for eXclusive Page Frame Ownership (XPFO) X-Virus-Scanned: ClamAV using ClamSMTP Hi Yisheng, > On Tue, Sep 12, 2017 at 04:05:22PM +0800, Yisheng Xie wrote: > > IMO, before a page is allocated, it is in buddy system, which means it is free > > and no other 'map' on the page except direct map. Then if the page is allocated > > to user, XPFO should unmap the direct map. otherwise the ret2dir may works at > > this window before it is freed. Or maybe I'm still missing anything. > > I agree that it seems broken. I'm just not sure why the test doesn't > fail. It's certainly worth understanding. Ok, so I think what's going on is that the page *is* mapped and unmapped by the kernel as Juerg described, but only in certain cases. See prep_new_page(), which has the following: if (!free_pages_prezeroed() && (gfp_flags & __GFP_ZERO)) for (i = 0; i < (1 << order); i++) clear_highpage(page + i); clear_highpage() maps and unmaps the pages, so that's why xpfo works with this set. I tried with CONFIG_PAGE_POISONING_ZERO=y and page_poison=y, and the XPFO_READ_USER test does not fail, i.e. the read succeeds. So, I think we need to include this zeroing condition in xpfo_alloc_pages(), something like the patch below. Unfortunately, this fails to boot for me, probably for an unrelated reason that I'll look into. Thanks a lot! Tycho From bfc21a6438cf8c56741af94cac939f1b0f63752c Mon Sep 17 00:00:00 2001 From: Tycho Andersen Date: Tue, 12 Sep 2017 12:06:41 -0600 Subject: [PATCH] draft of unmapping patch Signed-off-by: Tycho Andersen --- include/linux/xpfo.h | 5 +++-- mm/compaction.c | 2 +- mm/internal.h | 2 +- mm/page_alloc.c | 10 ++++++---- mm/xpfo.c | 10 ++++++++-- 5 files changed, 19 insertions(+), 10 deletions(-) diff --git a/include/linux/xpfo.h b/include/linux/xpfo.h index b24be9ac4a2d..c991bf7f051d 100644 --- a/include/linux/xpfo.h +++ b/include/linux/xpfo.h @@ -29,7 +29,7 @@ void xpfo_flush_kernel_tlb(struct page *page, int order); void xpfo_kmap(void *kaddr, struct page *page); void xpfo_kunmap(void *kaddr, struct page *page); -void xpfo_alloc_pages(struct page *page, int order, gfp_t gfp); +void xpfo_alloc_pages(struct page *page, int order, gfp_t gfp, bool will_map); void xpfo_free_pages(struct page *page, int order); bool xpfo_page_is_unmapped(struct page *page); @@ -49,7 +49,8 @@ void xpfo_temp_unmap(const void *addr, size_t size, void **mapping, static inline void xpfo_kmap(void *kaddr, struct page *page) { } static inline void xpfo_kunmap(void *kaddr, struct page *page) { } -static inline void xpfo_alloc_pages(struct page *page, int order, gfp_t gfp) { } +static inline void xpfo_alloc_pages(struct page *page, int order, gfp_t gfp, + bool will_map) { } static inline void xpfo_free_pages(struct page *page, int order) { } static inline bool xpfo_page_is_unmapped(struct page *page) { return false; } diff --git a/mm/compaction.c b/mm/compaction.c index fb548e4c7bd4..9a222258e65c 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -76,7 +76,7 @@ static void map_pages(struct list_head *list) order = page_private(page); nr_pages = 1 << order; - post_alloc_hook(page, order, __GFP_MOVABLE); + post_alloc_hook(page, order, __GFP_MOVABLE, false); if (order) split_page(page, order); diff --git a/mm/internal.h b/mm/internal.h index 4ef49fc55e58..1a0331ec2b2d 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -165,7 +165,7 @@ extern void __free_pages_bootmem(struct page *page, unsigned long pfn, unsigned int order); extern void prep_compound_page(struct page *page, unsigned int order); extern void post_alloc_hook(struct page *page, unsigned int order, - gfp_t gfp_flags); + gfp_t gfp_flags, bool will_map); extern int user_min_free_kbytes; #if defined CONFIG_COMPACTION || defined CONFIG_CMA diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 09fdf1bad21f..f73809847c58 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1750,7 +1750,7 @@ static bool check_new_pages(struct page *page, unsigned int order) } inline void post_alloc_hook(struct page *page, unsigned int order, - gfp_t gfp_flags) + gfp_t gfp_flags, bool will_map) { set_page_private(page, 0); set_page_refcounted(page); @@ -1759,18 +1759,20 @@ inline void post_alloc_hook(struct page *page, unsigned int order, kernel_map_pages(page, 1 << order, 1); kernel_poison_pages(page, 1 << order, 1); kasan_alloc_pages(page, order); - xpfo_alloc_pages(page, order, gfp_flags); + xpfo_alloc_pages(page, order, gfp_flags, will_map); set_page_owner(page, order, gfp_flags); } +extern bool xpfo_test; static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags, unsigned int alloc_flags) { int i; + bool needs_zero = !free_pages_prezeroed() && (gfp_flags & __GFP_ZERO); - post_alloc_hook(page, order, gfp_flags); + post_alloc_hook(page, order, gfp_flags, needs_zero); - if (!free_pages_prezeroed() && (gfp_flags & __GFP_ZERO)) + if (needs_zero) for (i = 0; i < (1 << order); i++) clear_highpage(page + i); diff --git a/mm/xpfo.c b/mm/xpfo.c index ca5d4d1838f9..dd25e24213fe 100644 --- a/mm/xpfo.c +++ b/mm/xpfo.c @@ -86,7 +86,7 @@ static inline struct xpfo *lookup_xpfo(struct page *page) return (void *)page_ext + page_xpfo_ops.offset; } -void xpfo_alloc_pages(struct page *page, int order, gfp_t gfp) +void xpfo_alloc_pages(struct page *page, int order, gfp_t gfp, bool will_map) { int i, flush_tlb = 0; struct xpfo *xpfo; @@ -116,8 +116,14 @@ void xpfo_alloc_pages(struct page *page, int order, gfp_t gfp) * Tag the page as a user page and flush the TLB if it * was previously allocated to the kernel. */ - if (!test_and_set_bit(XPFO_PAGE_USER, &xpfo->flags)) + bool was_user = !test_and_set_bit(XPFO_PAGE_USER, + &xpfo->flags); + + if (was_user || !will_map) { + set_kpte(page_address(page + i), page + i, + __pgprot(0)); flush_tlb = 1; + } } else { /* Tag the page as a non-user (kernel) page */ clear_bit(XPFO_PAGE_USER, &xpfo->flags);