From patchwork Thu Nov 1 03:23:54 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Xu X-Patchwork-Id: 991798 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.infradead.org (client-ip=2607:7c80:54:e::133; helo=bombadil.infradead.org; envelope-from=linux-snps-arc-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="GwHMGaXx"; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 42lrCp64Bnz9s3Z for ; Thu, 1 Nov 2018 14:24:22 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:Message-Id:Date: Subject:To:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To: References:List-Owner; bh=VHXOxXCBCrkk5llkMObwGWC/N5rw2dsheZU8Sr4qWPk=; b=GwH MGaXxBW1JQnDTy6gTXgKFftlUZBFUWFi3+mkek4NcXh7+bwDhNXm7kU+hshwGwwgdY6Vh7DmD2xoz oJZ59bHZO+YjW9bU8tYI+umAp2U3X1RkgT7fr/oosZjiH7wnJ6qsHDNBCj3soZbwOVE5aSmb4DnMt yn4Y4X2hFpDm2DYFpEYsTgvOqWJ2KlHgHe8lVC9wK1qZ0rmJjoPA/tSfLlDedQ8C/3fokXZr9XPDn px+SWV/Bf951yLXEfXtsSSF8x9kua84N9HOksj3IYgN5c5ZOi6Tet4ofr60btChbcTHzeeGFp6GK+ DM6dO1Zm9x1BK+4HaKiFXMmW+jRTnTA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gI3am-0002q5-P0; Thu, 01 Nov 2018 03:24:20 +0000 Received: from mx1.redhat.com ([209.132.183.28]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1gI3ae-0002of-6M for linux-snps-arc@lists.infradead.org; Thu, 01 Nov 2018 03:24:19 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 570E73001E49; Thu, 1 Nov 2018 03:24:00 +0000 (UTC) Received: from xz-x1.nay.redhat.com (dhcp-14-128.nay.redhat.com [10.66.14.128]) by smtp.corp.redhat.com (Postfix) with ESMTP id 958FF5D6A6; Thu, 1 Nov 2018 03:23:55 +0000 (UTC) From: Peter Xu To: linux-kernel@vger.kernel.org Subject: [PATCH RFC] mm: arc: fix potential double realease of mmap_sem Date: Thu, 1 Nov 2018 11:23:54 +0800 Message-Id: <20181101032354.19351-1-peterx@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Thu, 01 Nov 2018 03:24:00 +0000 (UTC) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20181031_202417_098336_6397036C X-CRM114-Status: GOOD ( 14.60 ) X-Spam-Score: -5.0 (-----) X-Spam-Report: SpamAssassin version 3.4.2 on bombadil.infradead.org summary: Content analysis details: (-5.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [209.132.183.28 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record X-BeenThere: linux-snps-arc@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Linux on Synopsys ARC Processors List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Andrea Arcangeli , Vineet Gupta , peterx@redhat.com, "Eric W. Biederman" , Andrew Morton , linux-snps-arc@lists.infradead.org, Souptick Joarder MIME-Version: 1.0 Sender: "linux-snps-arc" Errors-To: linux-snps-arc-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org In do_page_fault() of ARC we have: ... fault = handle_mm_fault(vma, address, flags); /* If Pagefault was interrupted by SIGKILL, exit page fault "early" */ if (unlikely(fatal_signal_pending(current))) { if ((fault & VM_FAULT_ERROR) && !(fault & VM_FAULT_RETRY)) up_read(&mm->mmap_sem); <---------------- [1] if (user_mode(regs)) return; } ... if (likely(!(fault & VM_FAULT_ERROR))) { ... return; } if (fault & VM_FAULT_OOM) goto out_of_memory; <----------------- [2] else if (fault & VM_FAULT_SIGSEGV) goto bad_area; <----------------- [3] else if (fault & VM_FAULT_SIGBUS) goto do_sigbus; <----------------- [4] Logically it's possible that we might try to release the mmap_sem twice by having a scenario like: - task received SIGKILL, - task handled kernel mode page fault, - handle_mm_fault() returned with one of VM_FAULT_ERROR, Then we'll go into path [1] to release the mmap_sem, however we won't return immediately since user_mode(regs) check will fail (a kernel page fault). Then we might go into either [2]-[4] and either of them will try to release the mmap_sem again. To fix this, we only release the mmap_sem at [1] when we're sure we'll quit immediately (after we checked with user_mode(regs)). CC: Vineet Gupta CC: "Eric W. Biederman" CC: Peter Xu CC: Andrew Morton CC: Souptick Joarder CC: Andrea Arcangeli CC: linux-snps-arc@lists.infradead.org CC: linux-kernel@vger.kernel.org Signed-off-by: Peter Xu --- I noticed this only by reading the code. Neither have I verified the issue, nor have I tested the patch since I even don't know how to (I'm totally unfamiliar with the arc architecture). However I'm posting this out first to see whether there's any quick feedback, and in case it's a valid issue that we've ignored. --- arch/arc/mm/fault.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index c9da6102eb4f..2d28c3dad5c1 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -142,11 +142,10 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) fault = handle_mm_fault(vma, address, flags); /* If Pagefault was interrupted by SIGKILL, exit page fault "early" */ - if (unlikely(fatal_signal_pending(current))) { - if ((fault & VM_FAULT_ERROR) && !(fault & VM_FAULT_RETRY)) + if (unlikely(fatal_signal_pending(current) && user_mode(regs))) { + if (!(fault & VM_FAULT_RETRY)) up_read(&mm->mmap_sem); - if (user_mode(regs)) - return; + return; } perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address);