From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754466AbdIHHIR (ORCPT ); Fri, 8 Sep 2017 03:08:17 -0400 Received: from smtp3-g21.free.fr ([212.27.42.3]:8605 "EHLO smtp3-g21.free.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753222AbdIHHIQ (ORCPT ); Fri, 8 Sep 2017 03:08:16 -0400 Subject: Re: [PATCH] powerpc/mm: Fix missing mmap_sem release To: Davidlohr Bueso Cc: benh@kernel.crashing.org, mpe@ellerman.id.au, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org References: <1504801529-15113-1-git-send-email-laurent.du4@free.fr> <20170907205146.GG17982@linux-80c1.suse> From: Laurent Dufour Message-ID: Date: Fri, 8 Sep 2017 09:08:02 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170907205146.GG17982@linux-80c1.suse> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/09/2017 22:51, Davidlohr Bueso wrote: > On Thu, 07 Sep 2017, Laurent Dufour wrote: > >> The commit b5c8f0fd595d ("powerpc/mm: Rework mm_fault_error()") reviewed >> the way the error path is managed in __do_page_fault() but it was a bit too >> agressive when handling a case by returning without releasing the mmap_sem. >> >> By the way, replacing current->mm->mmap_sem by mm->mmap_sem as mm is set to >> current->mm. >> >> Fixes: b5c8f0fd595d ("powerpc/mm: Rework mm_fault_error()") >> Cc: Benjamin Herrenschmidt >> Signed-off-by: Laurent Dufour >> --- >> arch/powerpc/mm/fault.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c >> index 4797d08581ce..f799ccf37d27 100644 >> --- a/arch/powerpc/mm/fault.c >> +++ b/arch/powerpc/mm/fault.c > > But... here: > > /* > * If we need to retry the mmap_sem has already been released, > * and if there is a fatal signal pending there is no guarantee > * that we made any progress. Handle this case first. > */ > >> @@ -521,10 +521,11 @@ static int __do_page_fault(struct pt_regs *regs, >> unsigned long address, >> * User mode? Just return to handle the fatal exception otherwise >> * return to bad_page_fault >> */ >> + up_read(&mm->mmap_sem); >> return is_user ? 0 : SIGBUS; >> } > > Per the above comment, for that case handle_mm_fault() > has already released mmap_sem. The same occurs in x86, > for example. Oops, my bad. Please forget about this stupid patch...