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=-11.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=no 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 A4A87C4727E for ; Thu, 1 Oct 2020 20:17:06 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 0ED7F20848 for ; Thu, 1 Oct 2020 20:17:05 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="cs9lvKVs" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0ED7F20848 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 3AED68E0001; Thu, 1 Oct 2020 16:17:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 35F396B006C; Thu, 1 Oct 2020 16:17:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 24EDF8E0001; Thu, 1 Oct 2020 16:17:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0146.hostedemail.com [216.40.44.146]) by kanga.kvack.org (Postfix) with ESMTP id E7AB06B0068 for ; Thu, 1 Oct 2020 16:17:04 -0400 (EDT) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 7ADF3180AD801 for ; Thu, 1 Oct 2020 20:17:04 +0000 (UTC) X-FDA: 77324465568.27.flag42_2108ec62719e Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin27.hostedemail.com (Postfix) with ESMTP id 57BB63D668 for ; Thu, 1 Oct 2020 20:17:04 +0000 (UTC) X-HE-Tag: flag42_2108ec62719e X-Filterd-Recvd-Size: 6621 Received: from mail-ej1-f67.google.com (mail-ej1-f67.google.com [209.85.218.67]) by imf29.hostedemail.com (Postfix) with ESMTP for ; Thu, 1 Oct 2020 20:17:03 +0000 (UTC) Received: by mail-ej1-f67.google.com with SMTP id nw23so9965154ejb.4 for ; Thu, 01 Oct 2020 13:17:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=KoOUahrMeyAsDE3oQQyqv2s9yY8l3xOTS/NeUkZg2MI=; b=cs9lvKVsNx/11Qr3REjeolUBHOiaVpoLUXk6f1C2zVadx1U7tV6p0ycccRnj0KUavS Y9TzZ4I3c726eRkbW2TGrgqYswDo6SfIP+38ukO+trgWznYHvEi3MvUiceLmBvibes8i ZtYTFjpMBsCyJjKB796uYMKj5L3uzo4LLgvjHtWcBeo1Tk4Yx+LFc+PqFfAKiE+o+2AS fora95yyPF8K5ifGMtGSbZ+C7u8C0sOCaimllNZzE61LAfI2/IQIVe0I2dxrhhiG5luz Q78dy/6lXP2MWiYmvHi3CEOkrGfwf7PdK2NUBcb6YJpZwrXx7c+E74tGnE6zBQwEddRG eGIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=KoOUahrMeyAsDE3oQQyqv2s9yY8l3xOTS/NeUkZg2MI=; b=qGrSuVilH7ey3rYCuQ32SZTqWGlURAMEJeKTzABhkiu2HrOP15Y9FthxzQoj8AWDGw kcJrmGsmk0R9ovJJqEM7+lXsAk51vFGlW7lenbpXFFJT/ozpunBrI/VmL/+x1d13MSHv qYpDoiiqEvqKR7eMEghkBl4TdyApYL+iqVzc9zAPthWRlJmkdKVDkU95HYWGi44HgiuD EoFYYs/8zF7NVFL2TeKLGNZmREMRXpQ08l12FzqBI4/nlpTy1VHzFQ8xSIv5u+3xH100 seWVUSvHM+4cMhI2O45yVxCRwuggKW5fWiDyT67o1d4lsspW0mej7hsW8Tz1/LARfGyX 7Qnw== X-Gm-Message-State: AOAM5335hrXBKvAXmhqqzfH/r5XtHfp3mQTUCLkw1M9cMInrf/bzPTzU dq7noIFi+NsbNcyMbf9chOdj1ESwqb/NtU3w/afvEQ== X-Google-Smtp-Source: ABdhPJxEM1ybNkLizKQYPuPD+Hu/qotsVG407dmmyfWsSnoQftdD93KL2bi2VTCAZfipItJRNzt7iQm9QVdZV7XDK9w= X-Received: by 2002:a17:907:94cf:: with SMTP id dn15mr10231781ejc.114.1601583422367; Thu, 01 Oct 2020 13:17:02 -0700 (PDT) MIME-Version: 1.0 References: <20200930011944.19869-1-jannh@google.com> <20200930123000.GC9916@ziepe.ca> <20200930232655.GE9916@ziepe.ca> <20201001191512.GF9916@ziepe.ca> In-Reply-To: <20201001191512.GF9916@ziepe.ca> From: Jann Horn Date: Thu, 1 Oct 2020 22:16:35 +0200 Message-ID: Subject: Re: [PATCH 3/4] mmap locking API: Don't check locking if the mm isn't live yet To: Jason Gunthorpe Cc: Michel Lespinasse , Andrew Morton , Linux-MM , kernel list , "Eric W . Biederman" , Mauro Carvalho Chehab , Sakari Ailus Content-Type: text/plain; charset="UTF-8" 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 Thu, Oct 1, 2020 at 9:15 PM Jason Gunthorpe wrote: > On Thu, Oct 01, 2020 at 01:51:33AM +0200, Jann Horn wrote: > > On Thu, Oct 1, 2020 at 1:26 AM Jason Gunthorpe wrote: > > > On Wed, Sep 30, 2020 at 10:14:57PM +0200, Jann Horn wrote: > > > > On Wed, Sep 30, 2020 at 2:50 PM Jann Horn wrote: > > > > > On Wed, Sep 30, 2020 at 2:30 PM Jason Gunthorpe wrote: > > > > > > On Tue, Sep 29, 2020 at 06:20:00PM -0700, Jann Horn wrote: > > > > > > > In preparation for adding a mmap_assert_locked() check in > > > > > > > __get_user_pages(), teach the mmap_assert_*locked() helpers that it's fine > > > > > > > to operate on an mm without locking in the middle of execve() as long as > > > > > > > it hasn't been installed on a process yet. > > > > > > > > > > > > I'm happy to see lockdep being added here, but can you elaborate on > > > > > > why add this mmap_locked_required instead of obtaining the lock in the > > > > > > execv path? > > > > > > > > > > My thinking was: At that point, we're logically still in the > > > > > single-owner initialization phase of the mm_struct. Almost any object > > > > > has initialization and teardown steps that occur in a context where > > > > > the object only has a single owner, and therefore no locking is > > > > > required. It seems to me that adding locking in places like > > > > > get_arg_page() would be confusing because it would suggest the > > > > > existence of concurrency where there is no actual concurrency, and it > > > > > might be annoying in terms of lockdep if someone tries to use > > > > > something like get_arg_page() while holding the mmap_sem of the > > > > > calling process. It would also mean that we'd be doing extra locking > > > > > in normal kernel builds that isn't actually logically required. > > > > > > > > > > Hmm, on the other hand, dup_mmap() already locks the child mm (with > > > > > mmap_write_lock_nested()), so I guess it wouldn't be too bad to also > > > > > do it in get_arg_page() and tomoyo_dump_page(), with comments that > > > > > note that we're doing this for lockdep consistency... I guess I can go > > > > > change this in v2. > > > > > > > > Actually, I'm taking that back. There's an extra problem: > > > > get_arg_page() accesses bprm->vma, which is set all the way back in > > > > __bprm_mm_init(). We really shouldn't be pretending that we're > > > > properly taking the mmap_sem when actually, we keep reusing a > > > > vm_area_struct pointer. > > > > > > Any chance the mmap lock can just be held from mm_struct allocation > > > till exec inserts it into the process? > > > > Hm... it should work if we define a lockdep subclass for this so that > > lockdep is happy when we call get_user() on the old mm_struct while > > holding that mmap lock. > > A subclass isn't right, it has to be a _nested annotation. > > nested locking is a pretty good reason to not be able to do this, this > is something lockdep does struggle to model. Did I get the terminology wrong? I thought they were the same. The down_*_nested() APIs take an argument "subclass", with the default subclass for the functions without "_nested" being 0. Anyway, I wrote a patch for this yesterday, I'll send it out later today after testing that it still boots without lockdep warnings. Then you can decide whether you prefer it to the current patch.