From patchwork Tue Nov 14 06:46:25 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rabin Vincent X-Patchwork-Id: 10057067 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 10E1460231 for ; Tue, 14 Nov 2017 06:47:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3841028E6B for ; Tue, 14 Nov 2017 06:47:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2B81828F60; Tue, 14 Nov 2017 06:47:05 +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.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 8154628E6B for ; Tue, 14 Nov 2017 06:47:04 +0000 (UTC) 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:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Pp7R+sGQXWj4KlQCB7MSvKEIgm4CHWUJS+EjfGLQPUY=; b=JY062UvzcAgcek S9bTUh58pjQCsYGHSLSQieWNLannqTkNkwX/9SOZ/psPGdGgDjlgITlvOQrgKiwbio2Dz5NJ5GwUC lS/XinB2ZYB74j8CXreSIRLJiTop7t6d5w0iwCiFPlVYnEZk778eZN9tJJcy9/qwUkvp23zm5ZujE 7cjKeDWzdT/wiIPv03yy+Df4fKegGbwjWk/nFMqZsN8ud/Mfbak6uR0EbFiioMoMB1sFMaxLaOKpc k179oQtqDsippuahENn9OnqCj+3yVh88rnGLBbeCoJ7iBlGBGvAfkVLxp3kB55hvgx7UlpV13Gf03 CBNIQy/Niro3QQcW9e7g==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1eEUzq-0003vY-VU; Tue, 14 Nov 2017 06:46:59 +0000 Received: from mail-lf0-x244.google.com ([2a00:1450:4010:c07::244]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1eEUzm-0003dg-Hj for linux-arm-kernel@lists.infradead.org; Tue, 14 Nov 2017 06:46:56 +0000 Received: by mail-lf0-x244.google.com with SMTP id s16so20783258lfs.1 for ; Mon, 13 Nov 2017 22:46:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=kX0mVMMpg1/u3RvBJHAcTMsOcVciGZJ7PxxiWbaTO6s=; b=QOx23IYoOatfqjikrbhUO2GJYfz9rMSauglxx6tYzFOsF15hVnXfm8nUG+RbGI+fSA MDho/9IapQNwkN11jnUDikwQwwEQJMc1gVZSv+f0I9mW0hY+A+VTmHnFA1mQWFs+WP0u PkFGgfBMHkOTlehNyE580EKft8lCj4Zq3ii6QC/7fHESniYCiyRfAG1ARdopGc4cU/WZ /2dMzZTzYyPTf2yfJwjBChkmeFEK+H1/936wX43XOhljV2kCE4SPqAB/TaArPJk8NmhC teNtovHBMBZFFfXC2DETmrJ56l0v5rDodHubkIPJ34howhNLxSbUSx1OOf7ue3Kgtpmm fVpA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=kX0mVMMpg1/u3RvBJHAcTMsOcVciGZJ7PxxiWbaTO6s=; b=AilQoAZeB61rHwO7sFqZc/s6FbvOFltDgnrV9m20xcLlZCfTfzs08pF609En5H80xY /zjBeeVUZOGAl3+tba52RFC/xmMhHw2BoOohz2nOyW1t03B8KhgHeA6zaCOOdomxlYN0 z18e6o+Uuz3RrbOi9wkUkX/HMSb92wZaOMAK+gbJLPl+E66h0lMM8HrFjwZt3gbjoCIf T/A0JkWcPU/2vRykA7Vhg3sb3i8WYmBnnR4UuIv+5cLSVsABRZo+NmtLe6BuKhewmD+2 mcG0hX1p+pg110JJObbubUtH1BNTu/FDhlIdqXOKCE/Zr2L02Rpr3TboIw7jOg6yg7OO Ar9Q== X-Gm-Message-State: AJaThX78ccbm+ewIOsG37PvTxIzRraNJqqbvTPo+hkPV55nrHSdVbRLg qKp417KSK6ngZgRr/PdfDgo= X-Google-Smtp-Source: AGs4zMao+NM+Fp5pP1xiNPNgbPUwS9X2Nl3OFDTe1CR38IZTEkqZzEbjNyBhQ1xw5M5+qRS9zor9nw== X-Received: by 10.25.29.78 with SMTP id d75mr3116253lfd.39.1510641991937; Mon, 13 Nov 2017 22:46:31 -0800 (PST) Received: from lnxartpec.se.axis.com (proxy02.se.axis.com. [195.60.68.157]) by smtp.gmail.com with ESMTPSA id u11sm736961ljd.70.2017.11.13.22.46.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 13 Nov 2017 22:46:28 -0800 (PST) Date: Tue, 14 Nov 2017 07:46:25 +0100 From: Rabin Vincent To: Will Deacon Subject: Re: [PATCH 1/2] arm64: mm: abort uaccess retries upon fatal signal Message-ID: <20171114064556.GA18291@lnxartpec.se.axis.com> References: <1499782763-31418-1-git-send-email-mark.rutland@arm.com> <1499782763-31418-2-git-send-email-mark.rutland@arm.com> <20170711145849.GE13977@arm.com> <20170821134202.GA5418@leverpostej> <20170822094523.GA5439@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170822094523.GA5439@arm.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171113_224654_786678_2AE4460D X-CRM114-Status: GOOD ( 41.25 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , linux-arch@vger.kernel.org, steve.capper@arm.com, peterz@infradead.org, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, stable@vger.kernel.org, linux@armlinux.org.uk, luto@amacapital.net, james.morse@arm.com, viro@zeniv.linux.org.uk, labbott@redhat.com, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Aug 22, 2017 at 10:45:24AM +0100, Will Deacon wrote: > On Mon, Aug 21, 2017 at 02:42:03PM +0100, Mark Rutland wrote: > > On Tue, Jul 11, 2017 at 03:58:49PM +0100, Will Deacon wrote: > > > On Tue, Jul 11, 2017 at 03:19:22PM +0100, Mark Rutland wrote: > > > > When there's a fatal signal pending, arm64's do_page_fault() > > > > implementation returns 0. The intent is that we'll return to the > > > > faulting userspace instruction, delivering the signal on the way. > > > > > > > > However, if we take a fatal signal during fixing up a uaccess, this > > > > results in a return to the faulting kernel instruction, which will be > > > > instantly retried, resulting in the same fault being taken forever. As > > > > the task never reaches userspace, the signal is not delivered, and the > > > > task is left unkillable. While the task is stuck in this state, it can > > > > inhibit the forward progress of the system. > > > > > > > > To avoid this, we must ensure that when a fatal signal is pending, we > > > > apply any necessary fixup for a faulting kernel instruction. Thus we > > > > will return to an error path, and it is up to that code to make forward > > > > progress towards delivering the fatal signal. > > > > > > > > Signed-off-by: Mark Rutland > > > > Reviewed-by: Steve Capper > > > > Tested-by: Steve Capper > > > > Cc: Catalin Marinas > > > > Cc: James Morse > > > > Cc: Laura Abbott > > > > Cc: Will Deacon > > > > Cc: stable@vger.kernel.org > > > > --- > > > > arch/arm64/mm/fault.c | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > > > > index 37b95df..3952d5e 100644 > > > > --- a/arch/arm64/mm/fault.c > > > > +++ b/arch/arm64/mm/fault.c > > > > @@ -397,8 +397,11 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr, > > > > * signal first. We do not need to release the mmap_sem because it > > > > * would already be released in __lock_page_or_retry in mm/filemap.c. > > > > */ > > > > - if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current)) > > > > + if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current)) { > > > > + if (!user_mode(regs)) > > > > + goto no_context; > > > > return 0; > > > > + } > > > > > > This will need rebasing at -rc1 (take a look at current HEAD). > > > > > > Also, I think it introduces a weird corner case where we take a page fault > > > when writing the signal frame to the user stack to deliver a SIGSEGV. If > > > we end up with VM_FAULT_RETRY and somebody has sent a SIGKILL to the task, > > > then we'll fail setup_sigframe and force an un-handleable SIGSEGV instead > > > of SIGKILL. > > > > > > The end result (task is killed) is the same, but the fatal signal is wrong. > > > > That doesn't seem to be the case, testing on v4.13-rc5. > > > > I used sigaltstack() to use the userfaultfd region as signal stack, > > registerd a SIGSEGV handler, and dereferenced NULL. The task locks up, > > but when killed with a SIGINT or SIGKILL, the exit status reflects that > > signal, rather than the SIGSEGV. > > > > If I move the SIGINT handler onto the userfaultfd-monitored stack, then > > delivering SIGINT hangs, but can be killed with SIGKILL, and the exit > > status reflects that SIGKILL. > > > > As you say, it does look like we'd try to set up a deferred SIGSEGV for > > the failed signal delivery. > > > > I haven't yet figured out exactly how that works; I'll keep digging. > > The SEGV makes it all the way into do_group_exit, but then signal_group_exit > is set and the exit_code is overridden with SIGKILL at the last minute (see > complete_signal). Unfortunately, this last minute is too late for print-fatal-signals. With print-fatal-signals enabled, this patch leads to misleading "potentially unexpected fatal signal 11" warnings if a process is SIGKILL'd at the right time. I've seen it without userfaultfd, but it's easiest reproduced by patching Mark's original test code [1] with the following patch and simply running "pkill -WINCH foo; pkill -KILL foo". This results in: foo: potentially unexpected fatal signal 11. CPU: 1 PID: 1793 Comm: foo Not tainted 4.9.58-devel #3 task: b3534780 task.stack: b4b7c000 PC is at 0x76effb60 LR is at 0x4227f4 pc : [<76effb60>] lr : [<004227f4>] psr: 600b0010 sp : 7eaf7bb4 ip : 00000000 fp : 00000000 r10: 00000001 r9 : 00000003 r8 : 76fcd000 r7 : 0000001d r6 : 76fd0cf0 r5 : 7eaf7c08 r4 : 00000000 r3 : 00000000 r2 : 00000000 r1 : 7eaf7a88 r0 : fffffffc Flags: nZCv IRQs on FIQs on Mode USER_32 ISA ARM Segment user Control: 10c5387d Table: 3357404a DAC: 00000055 CPU: 1 PID: 1793 Comm: foo Not tainted 4.9.58-devel #3 [<801113f0>] (unwind_backtrace) from [<8010cfb0>] (show_stack+0x18/0x1c) [<8010cfb0>] (show_stack) from [<8039725c>] (dump_stack+0x84/0x98) [<8039725c>] (dump_stack) from [<8012f448>] (get_signal+0x384/0x684) [<8012f448>] (get_signal) from [<8010c2ec>] (do_signal+0xcc/0x470) [<8010c2ec>] (do_signal) from [<8010c868>] (do_work_pending+0xb8/0xc8) [<8010c868>] (do_work_pending) from [<801086d4>] (slow_work_pending+0xc/0x20) This is ARM and I haven't tested ARM64, but the same problem even exists on x86. [1] https://lkml.kernel.org/r/1499782590-31366-1-git-send-email-mark.rutland@arm.com --- foo.c.orig 2017-11-13 23:45:47.802167284 +0100 +++ foo.c 2017-11-14 07:16:13.906363466 +0100 @@ -6,6 +6,11 @@ #include #include #include +#include + +static void handler(int sig) +{ +} int main(int argc, char *argv[]) { @@ -47,13 +52,17 @@ if (ret < 0) return errno; + sigaltstack(&(stack_t){.ss_sp = mem, .ss_size = pagesz}, NULL); + sigaction(SIGWINCH, &(struct sigaction){ .sa_handler = handler, .sa_flags = SA_ONSTACK, }, NULL); + /* * Force an arbitrary uaccess to memory monitored by the userfaultfd. * This will block, but when a SIGKILL is sent, will consume all * available CPU time without being killed, and may inhibit forward * progress of the system. */ - ret = fstatfs(0, (struct statfs *)mem); + // ret = fstatfs(0, (struct statfs *)mem); + pause(); return 0; }