From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4F15CC43461 for ; Mon, 14 Sep 2020 10:56:34 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 80BFA208DB for ; Mon, 14 Sep 2020 10:56:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="MypNEzMC" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 80BFA208DB Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B42F16B0037; Mon, 14 Sep 2020 06:56:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AF3C76B0055; Mon, 14 Sep 2020 06:56:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A0AD96B005A; Mon, 14 Sep 2020 06:56:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0061.hostedemail.com [216.40.44.61]) by kanga.kvack.org (Postfix) with ESMTP id 8C1596B0037 for ; Mon, 14 Sep 2020 06:56:32 -0400 (EDT) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 4BA4783A7C1B for ; Mon, 14 Sep 2020 10:56:32 +0000 (UTC) X-FDA: 77261363424.05.sink17_281569a27108 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin05.hostedemail.com (Postfix) with ESMTP id 1EF7A180D0FBB for ; Mon, 14 Sep 2020 10:56:32 +0000 (UTC) X-HE-Tag: sink17_281569a27108 X-Filterd-Recvd-Size: 6654 Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by imf34.hostedemail.com (Postfix) with ESMTP for ; Mon, 14 Sep 2020 10:56:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=merlin.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=12iDO9zK0M5MrFc9v9LnmYPbTArC0uf4AF99UCMX0/g=; b=MypNEzMCKsV0kDjFZpdYlzLbrc 6jVvG6vst2kP1TuKPU0RbKSwT4f5aXkemcmA++NmHlfjANB+KOIuYBTFK+U4dDrGkq08pVvTYuCGr 64XrtFpwD+zaYM0wPLqkolL1pXzmez6O2GauA4GZxoZWC2JUCo58zMGjmZ4KQyykyxoC3pE4HFsOV wOdaJZ4wz5kYT0uT7MPzM6GQxyCQ6sVHR+Fiqkx1/HH5YcgPYsMoXksVoBOfa6PPoTtsC/17phU5P 0FBTRJC5a6qRi0OxEvGOoCLGZsWCVmH7DaTXnCWgOL7BmL6M13ECbEyckAnfF5siMO3wHN+5pxlQD k7eINU4g==; Received: from j217100.upc-j.chello.nl ([24.132.217.100] helo=noisy.programming.kicks-ass.net) by merlin.infradead.org with esmtpsa (Exim 4.92.3 #3 (Red Hat Linux)) id 1kHm9l-0001Ya-CT; Mon, 14 Sep 2020 10:56:21 +0000 Received: from hirez.programming.kicks-ass.net (hirez.programming.kicks-ass.net [192.168.1.225]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by noisy.programming.kicks-ass.net (Postfix) with ESMTPS id 8D20A30257C; Mon, 14 Sep 2020 12:56:17 +0200 (CEST) Received: by hirez.programming.kicks-ass.net (Postfix, from userid 1000) id 694F52CA75A81; Mon, 14 Sep 2020 12:56:17 +0200 (CEST) Date: Mon, 14 Sep 2020 12:56:17 +0200 From: peterz@infradead.org To: Nicholas Piggin Cc: "linux-mm @ kvack . org" , linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, sparclinux@vger.kernel.org, "Aneesh Kumar K . V" , Andrew Morton , Jens Axboe , "David S . Miller" , Andy Lutomirski , Dave Hansen Subject: Re: [PATCH v2 1/4] mm: fix exec activate_mm vs TLB shootdown and lazy tlb switching race Message-ID: <20200914105617.GP1362448@hirez.programming.kicks-ass.net> References: <20200914045219.3736466-1-npiggin@gmail.com> <20200914045219.3736466-2-npiggin@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200914045219.3736466-2-npiggin@gmail.com> X-Rspamd-Queue-Id: 1EF7A180D0FBB X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam02 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Sep 14, 2020 at 02:52:16PM +1000, Nicholas Piggin wrote: > Reading and modifying current->mm and current->active_mm and switching > mm should be done with irqs off, to prevent races seeing an intermediate > state. > > This is similar to commit 38cf307c1f20 ("mm: fix kthread_use_mm() vs TLB > invalidate"). At exec-time when the new mm is activated, the old one > should usually be single-threaded and no longer used, unless something > else is holding an mm_users reference (which may be possible). > > Absent other mm_users, there is also a race with preemption and lazy tlb > switching. Consider the kernel_execve case where the current thread is > using a lazy tlb active mm: > > call_usermodehelper() > kernel_execve() > old_mm = current->mm; > active_mm = current->active_mm; > *** preempt *** --------------------> schedule() > prev->active_mm = NULL; > mmdrop(prev active_mm); > ... > <-------------------- schedule() > current->mm = mm; > current->active_mm = mm; > if (!old_mm) > mmdrop(active_mm); > > If we switch back to the kernel thread from a different mm, there is a > double free of the old active_mm, and a missing free of the new one. > > Closing this race only requires interrupts to be disabled while ->mm > and ->active_mm are being switched, but the TLB problem requires also > holding interrupts off over activate_mm. Unfortunately not all archs > can do that yet, e.g., arm defers the switch if irqs are disabled and > expects finish_arch_post_lock_switch() to be called to complete the > flush; um takes a blocking lock in activate_mm(). > > So as a first step, disable interrupts across the mm/active_mm updates > to close the lazy tlb preempt race, and provide an arch option to > extend that to activate_mm which allows architectures doing IPI based > TLB shootdowns to close the second race. > > This is a bit ugly, but in the interest of fixing the bug and backporting > before all architectures are converted this is a compromise. > > Signed-off-by: Nicholas Piggin Acked-by: Peter Zijlstra (Intel) I'm thinking we want this selected on x86 as well. Andy? > --- > arch/Kconfig | 7 +++++++ > fs/exec.c | 17 +++++++++++++++-- > 2 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/arch/Kconfig b/arch/Kconfig > index af14a567b493..94821e3f94d1 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -414,6 +414,13 @@ config MMU_GATHER_NO_GATHER > bool > depends on MMU_GATHER_TABLE_FREE > > +config ARCH_WANT_IRQS_OFF_ACTIVATE_MM > + bool > + help > + Temporary select until all architectures can be converted to have > + irqs disabled over activate_mm. Architectures that do IPI based TLB > + shootdowns should enable this. > + > config ARCH_HAVE_NMI_SAFE_CMPXCHG > bool > > diff --git a/fs/exec.c b/fs/exec.c > index a91003e28eaa..d4fb18baf1fb 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1130,11 +1130,24 @@ static int exec_mmap(struct mm_struct *mm) > } > > task_lock(tsk); > - active_mm = tsk->active_mm; > membarrier_exec_mmap(mm); > - tsk->mm = mm; > + > + local_irq_disable(); > + active_mm = tsk->active_mm; > tsk->active_mm = mm; > + tsk->mm = mm; > + /* > + * This prevents preemption while active_mm is being loaded and > + * it and mm are being updated, which could cause problems for > + * lazy tlb mm refcounting when these are updated by context > + * switches. Not all architectures can handle irqs off over > + * activate_mm yet. > + */ > + if (!IS_ENABLED(CONFIG_ARCH_WANT_IRQS_OFF_ACTIVATE_MM)) > + local_irq_enable(); > activate_mm(active_mm, mm); > + if (IS_ENABLED(CONFIG_ARCH_WANT_IRQS_OFF_ACTIVATE_MM)) > + local_irq_enable(); > tsk->mm->vmacache_seqnum = 0; > vmacache_flush(tsk); > task_unlock(tsk); > -- > 2.23.0 >