From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1167662AbdDXJPi (ORCPT ); Mon, 24 Apr 2017 05:15:38 -0400 Received: from ozlabs.org ([103.22.144.67]:53397 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1167748AbdDXJP0 (ORCPT ); Mon, 24 Apr 2017 05:15:26 -0400 From: Michael Ellerman To: Christophe Leroy , Benjamin Herrenschmidt , Paul Mackerras , Scott Wood Cc: linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH] powerpc/mm: Only read faulting instruction when necessary in do_page_fault() In-Reply-To: <20170424082832.C0E8C6EAF5@localhost.localdomain> References: <20170424082832.C0E8C6EAF5@localhost.localdomain> User-Agent: Notmuch/0.21 (https://notmuchmail.org) Date: Mon, 24 Apr 2017 19:15:20 +1000 Message-ID: <87d1c2rwt3.fsf@concordia.ellerman.id.au> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Christophe Leroy writes: > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c > index 400f2d0d42f8..3d506589236c 100644 > --- a/arch/powerpc/mm/fault.c > +++ b/arch/powerpc/mm/fault.c > @@ -356,8 +348,22 @@ int do_page_fault(struct pt_regs *regs, unsigned long address, > * between the last mapped region and the stack will > * expand the stack rather than segfaulting. > */ > - if (address + 2048 < uregs->gpr[1] && !store_updates_sp(inst)) > - goto bad_area; > + if (address + 2048 < uregs->gpr[1]) { > + if (!inst) { That looks like it could lead to an infinite loop, if the instruction that caused the fault has been overwritten with NULL by another thread. Or did I miss something? (I only read the diff) > + /* > + * We want to do this outside mmap_sem, because > + * reading code around nip can result in fault, > + * which will cause a deadlock when called with > + * mmap_sem held > + */ > + up_read(&mm->mmap_sem); > + __get_user(inst, > + (unsigned int __user *)regs->nip); > + if (!store_updates_sp(inst)) > + goto bad_area_nosemaphore; > + goto retry; > + } cheers