From mboxrd@z Thu Jan 1 00:00:00 1970 From: Borislav Petkov Subject: Re: [PATCH v4 04/29] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated Date: Tue, 28 Jun 2016 20:48:10 +0200 Message-ID: <20160628184810.GA2855@pd.tnic> References: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Return-path: Received: from mail.skyhub.de ([78.46.96.112]:35554 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751158AbcF1S7v (ORCPT ); Tue, 28 Jun 2016 14:59:51 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-arch-owner@vger.kernel.org List-ID: To: Andy Lutomirski Cc: x86@kernel.org, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, Nadav Amit , Kees Cook , Brian Gerst , "kernel-hardening@lists.openwall.com" , Linus Torvalds , Josh Poimboeuf , Jann Horn , Heiko Carstens On Sun, Jun 26, 2016 at 02:55:26PM -0700, Andy Lutomirski wrote: > This avoids pointless races in which another CPU or task might see a > partially populated global pgd entry. These races should normally > be harmless, but, if another CPU propagates the entry via > vmalloc_fault and then populate_pgd fails (due to memory allocation > failure, for example), this prevents a use-after-free of the pgd > entry. > > Signed-off-by: Andy Lutomirski > --- > arch/x86/mm/pageattr.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c > index 7a1f7bbf4105..6a8026918bf6 100644 > --- a/arch/x86/mm/pageattr.c > +++ b/arch/x86/mm/pageattr.c > @@ -1113,7 +1113,9 @@ static int populate_pgd(struct cpa_data *cpa, unsigned long addr) > > ret = populate_pud(cpa, addr, pgd_entry, pgprot); > if (ret < 0) { > - unmap_pgd_range(cpa->pgd, addr, > + if (pud) > + free_page((unsigned long)pud); > + unmap_pud_range(pgd_entry, addr, > addr + (cpa->numpages << PAGE_SHIFT)); > return ret; > } > -- So something's amiss here. Subject says: "x86/cpa: In populate_pgd, don't set the pgd entry until it's populated" but you haven't moved set_pgd(pgd_entry, __pgd(__pa(pud) | _KERNPG_TABLE)); after populate_pud() succeeds... Which is a good catch but your patch should do it too. :-) -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply.