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=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 E4875C4338F for ; Tue, 3 Aug 2021 23:35:46 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 4379060EE8 for ; Tue, 3 Aug 2021 23:35:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4379060EE8 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ziepe.ca Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id BBF438D0010; Tue, 3 Aug 2021 19:35:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B6F3F8D000F; Tue, 3 Aug 2021 19:35:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A366C8D0010; Tue, 3 Aug 2021 19:35:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0195.hostedemail.com [216.40.44.195]) by kanga.kvack.org (Postfix) with ESMTP id 882158D000F for ; Tue, 3 Aug 2021 19:35:45 -0400 (EDT) Received: from smtpin38.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 2C00C181DF740 for ; Tue, 3 Aug 2021 23:35:45 +0000 (UTC) X-FDA: 78435379050.38.EAD0949 Received: from mail-qk1-f170.google.com (mail-qk1-f170.google.com [209.85.222.170]) by imf18.hostedemail.com (Postfix) with ESMTP id B3EAE400330A for ; Tue, 3 Aug 2021 23:35:44 +0000 (UTC) Received: by mail-qk1-f170.google.com with SMTP id o13so1097788qkk.9 for ; Tue, 03 Aug 2021 16:35:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ziepe.ca; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=uhDyUj2VSVRpxbG/0UXwj6BIhNgU6btFMLRW7jRZu+0=; b=kZtnicXFx9GD20ho5v8snd5tko753aUlUgTIuPa6FbiGiWZEJWWlvpZ+LQ8DqwRVMW tW/sbXcwp7Y00/DWrE8kE3MLU6XaxvcZQmjBeduRZm56uPe8Io+SoqspP6ERGOXZGXlP sMcMCUbCZl4uF7hFRclzuzwt3GcSQu5qp/w/3BwfR9pXXVP1cfr5PPTrCreq+C6D7Yj9 Vq17Iaymneru9n4bdT/CJxdAu4/2oF6aIFlTFu0RTyIgkvxZ8ZhfJ0CZXtfpwn2p4JzW FhEn9esnkhvsSGsZvkfYjZEd+dqK3fACCSHC5yoB2kHUg/Brs6CtNpj6jujsECF1WHQl yCaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=uhDyUj2VSVRpxbG/0UXwj6BIhNgU6btFMLRW7jRZu+0=; b=Zh6H4PHfwkl25TmthmIAcdH65Wczdm04D4KPyPlxzO+Gd+cAQ1P5jSUx1WROAMuevQ J6CHYikY2yHb9jyGmB/KUpeRanGYpVZSdfTzj0XZhrvuXLmP5QrUzcVE/cmWiV8GeSMG qhUsTbQmk2mHQYRTXdzecO0G2Fi5YpqCQybW8UrJeqhkeMOxEPwE0JWWkpFqPNaDlFz9 Ot/NMVjQeMHoXtAoY07CqIJk0lZvwJ/Xtw2RbydoweTQ8q2JQjOabSW46I+dThxPxgUt MQz0vxFh6NDlx7DDH5UHWz8DHVKZj9fwoSshX98/jgFG2IlcfZhYDLeCkSH7igsBYSpI kHow== X-Gm-Message-State: AOAM531GEsPU5G22LM05f8HttCqb0x75xGLZ990ezYj/shGLnDERhrwv E1Jtt1D/p8YvtIUpVN11i7j2EA== X-Google-Smtp-Source: ABdhPJx/JXJh+CCyExIE/rFD7Gs85I8uz1IOyOfUCLsinNt7bQE3NPusvSSul/lc54NwuguJEWasSA== X-Received: by 2002:a05:620a:64c:: with SMTP id a12mr18105687qka.377.1628033744011; Tue, 03 Aug 2021 16:35:44 -0700 (PDT) Received: from ziepe.ca ([206.223.160.26]) by smtp.gmail.com with ESMTPSA id 28sm309576qkp.26.2021.08.03.16.35.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Aug 2021 16:35:43 -0700 (PDT) Received: from jgg by mlx with local (Exim 4.94) (envelope-from ) id 1mB3wk-00CMna-H4; Tue, 03 Aug 2021 20:35:42 -0300 Date: Tue, 3 Aug 2021 20:35:42 -0300 From: Jason Gunthorpe To: Liam Howlett Cc: Luigi Rizzo , Jann Horn , linux-kernel , Andrew Morton , David Rientjes , "linux-mm@kvack.org" Subject: Re: [PATCH] Add mmap_assert_locked() annotations to find_vma*() Message-ID: <20210803233542.GH543798@ziepe.ca> References: <20210731175341.3458608-1-lrizzo@google.com> <20210803160803.GG543798@ziepe.ca> <20210803230725.ao3i2emejyyor36n@revolver> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210803230725.ao3i2emejyyor36n@revolver> X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: B3EAE400330A Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=ziepe.ca header.s=google header.b=kZtnicXF; dmarc=none; spf=pass (imf18.hostedemail.com: domain of jgg@ziepe.ca designates 209.85.222.170 as permitted sender) smtp.mailfrom=jgg@ziepe.ca X-Stat-Signature: dz6x457dshtoeqd14ykzuryz5rmytppn X-HE-Tag: 1628033744-549845 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 Tue, Aug 03, 2021 at 11:07:35PM +0000, Liam Howlett wrote: > * Luigi Rizzo [210803 17:49]: > > On Tue, Aug 3, 2021 at 6:08 PM Jason Gunthorpe wrote: > > > > > > On Sat, Jul 31, 2021 at 10:53:41AM -0700, Luigi Rizzo wrote: > > > > find_vma() and variants need protection when used. > > > > This patch adds mmap_assert_lock() calls in the functions. > > > > > > > > To make sure the invariant is satisfied, we also need to add a > > > > mmap_read_loc() around the get_user_pages_remote() call in > > > > get_arg_page(). The lock is not strictly necessary because the mm > > > > has been newly created, but the extra cost is limited because > > > > the same mutex was also acquired shortly before in __bprm_mm_init(), > > > > so it is hot and uncontended. > > > > > > > > Signed-off-by: Luigi Rizzo > > > > fs/exec.c | 2 ++ > > > > mm/mmap.c | 2 ++ > > > > 2 files changed, 4 insertions(+) > > > > > > > > diff --git a/fs/exec.c b/fs/exec.c > > > > index 38f63451b928..ac7603e985b4 100644 > > > > +++ b/fs/exec.c > > > > @@ -217,8 +217,10 @@ static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, > > > > * We are doing an exec(). 'current' is the process > > > > * doing the exec and bprm->mm is the new process's mm. > > > > */ > > > > + mmap_read_lock(bprm->mm); > > > > ret = get_user_pages_remote(bprm->mm, pos, 1, gup_flags, > > > > &page, NULL, NULL); > > > > + mmap_read_unlock(bprm->mm); > > > > if (ret <= 0) > > > > return NULL; > > > > > > Wasn't Jann Horn working on something like this too? > > > > > > https://lore.kernel.org/linux-mm/20201016225713.1971256-1-jannh@google.com/ > > > > > > IIRC it was very tricky here, are you sure it is OK to obtain this lock > > > here? > > > > I cannot comment on Jann's patch series but no other thread knows > > about this mm at this point in the code so the lock is definitely > > safe to acquire (shortly before there was also a write lock acquired > > on the same mm, in the same conditions). > > If there is no other code that knows about this mm, then does one need > the lock at all? Is this just to satisfy the new check you added? > > If you want to make this change, I would suggest writing it in a way to > ensure the call to expand_downwards() in the same function also holds > the lock. I believe this is technically required as well? What do you > think? This is essentially what Jann was doing. Since the mm is newly created we can create it write locked and then we can add proper locking tests to many of the functions called along this path. Adding useless locks around each troublesome callsite just seems really confusing to me. Jason