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=-4.1 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 035F7C4727F for ; Wed, 30 Sep 2020 23:27:00 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7A1D0206F4 for ; Wed, 30 Sep 2020 23:26:59 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=ziepe.ca header.i=@ziepe.ca header.b="kJUsDmf3" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7A1D0206F4 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ziepe.ca Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id F24C16B005C; Wed, 30 Sep 2020 19:26:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id ED41D6B0062; Wed, 30 Sep 2020 19:26:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D9B236B0068; Wed, 30 Sep 2020 19:26:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0032.hostedemail.com [216.40.44.32]) by kanga.kvack.org (Postfix) with ESMTP id BFBD36B005C for ; Wed, 30 Sep 2020 19:26:58 -0400 (EDT) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 79E232C8B for ; Wed, 30 Sep 2020 23:26:58 +0000 (UTC) X-FDA: 77321315316.15.wire88_470b92827196 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin15.hostedemail.com (Postfix) with ESMTP id 53E441814FF0F for ; Wed, 30 Sep 2020 23:26:58 +0000 (UTC) X-HE-Tag: wire88_470b92827196 X-Filterd-Recvd-Size: 5950 Received: from mail-qt1-f195.google.com (mail-qt1-f195.google.com [209.85.160.195]) by imf37.hostedemail.com (Postfix) with ESMTP for ; Wed, 30 Sep 2020 23:26:57 +0000 (UTC) Received: by mail-qt1-f195.google.com with SMTP id d1so2826274qtr.6 for ; Wed, 30 Sep 2020 16:26:57 -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=QYFXqJkpWwNHWn45HDc4xPYLfZFQs73v+wcJibg2ncI=; b=kJUsDmf3lco5P+YD/j58UOEmKbo+GoiJA2V3Z1FFEczpRqtI8xR8187j30EqlUyi8Z Pym66gmusyN2Pt6z6t4Le10rbm8rf09VSyn4OdkTiK+9mXD2dvCc4/2xJE9kb3kH9W3Q 1RTawqwnwbfc/FQIe6qJQfccQdliHVY8RyEZkhwjStES2oaDzCZCddvRJMs5ew3bJn/E 3ZvO5CxVYbFBuEoi9zIDq3iLbedeelXF+PE3SigZIBih8QmCltG/heWCIfQbNLQiKJ0N jDaHT6Y7+iXHypDhX43ERuWIrbHCXdEjrowqFFE732q80p7Hupn3IXT0F/ldHk5t408e iVXw== 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=QYFXqJkpWwNHWn45HDc4xPYLfZFQs73v+wcJibg2ncI=; b=o2bC52W2QgdCeRorMn9R+o4AZOmEYUFNrd1ZQhGKAL9pKb9AVohGL6Y8JtaVib+Z1N YCDxnITGsjX0vBbihZUqTpn64rBRe8Xt9f/BbA9TmxS47oLTFcaXVlCyyg1boCXB0n4p SNz9MKLdDThcNMX7a9rCn26n041+4NohESpkHszIHtSumLnDRLGqZaqfFJ4d2rxzpiqG 7PstD3Pv7QnRuB0xqHQQI/F/0gLepiAf+Jcu49eDrU1N49zdeLyi9OWAc6ipifxF5nZD ihXEihKbdDkCRMaG/PY049AgzkJsm1Mi+uRvMvSrEiVhBJyZYKJqGwvx25DhJBaSmQp/ DBhA== X-Gm-Message-State: AOAM532g/o011WfoDn9VLiZFKvjMFVJJ5vkxaI6CSu848X0tGEMb4r6t gxw2As9lGnLrdvhUZanjn2RXKQ== X-Google-Smtp-Source: ABdhPJx0Ptq8Rn7jIQg8667tHirN8NeH8LBlxqTnmJRotl6mZoQ2FfpLkjbrx0nLyfZQAzU3XdbH7A== X-Received: by 2002:ac8:409e:: with SMTP id p30mr4768472qtl.208.1601508417240; Wed, 30 Sep 2020 16:26:57 -0700 (PDT) Received: from ziepe.ca (hlfxns017vw-156-34-48-30.dhcp-dynamic.fibreop.ns.bellaliant.net. [156.34.48.30]) by smtp.gmail.com with ESMTPSA id 8sm3982768qkc.100.2020.09.30.16.26.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Sep 2020 16:26:56 -0700 (PDT) Received: from jgg by mlx with local (Exim 4.94) (envelope-from ) id 1kNlUt-004PIm-R3; Wed, 30 Sep 2020 20:26:55 -0300 Date: Wed, 30 Sep 2020 20:26:55 -0300 From: Jason Gunthorpe To: Jann Horn Cc: Andrew Morton , Linux-MM , kernel list , "Eric W . Biederman" , Michel Lespinasse , Mauro Carvalho Chehab , Sakari Ailus Subject: Re: [PATCH 3/4] mmap locking API: Don't check locking if the mm isn't live yet Message-ID: <20200930232655.GE9916@ziepe.ca> References: <20200930011944.19869-1-jannh@google.com> <20200930123000.GC9916@ziepe.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 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? > Does that sound reasonable? My only concern is how weird it is to do this with a variable, I've never seen something like this before Jason