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,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 E0836C4727D for ; Sat, 3 Oct 2020 21:30:47 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5FD3220717 for ; Sat, 3 Oct 2020 21:30:47 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="sOuG2fbg" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5FD3220717 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 76CA46B005D; Sat, 3 Oct 2020 17:30:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 71DF56B0062; Sat, 3 Oct 2020 17:30:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 60CD66B0068; Sat, 3 Oct 2020 17:30:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0074.hostedemail.com [216.40.44.74]) by kanga.kvack.org (Postfix) with ESMTP id 330F16B005D for ; Sat, 3 Oct 2020 17:30:46 -0400 (EDT) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id C31163630 for ; Sat, 3 Oct 2020 21:30:45 +0000 (UTC) X-FDA: 77331908850.17.dirt00_62017af271b0 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin17.hostedemail.com (Postfix) with ESMTP id 814D0180D0186 for ; Sat, 3 Oct 2020 21:30:45 +0000 (UTC) X-HE-Tag: dirt00_62017af271b0 X-Filterd-Recvd-Size: 5960 Received: from mail-yb1-f194.google.com (mail-yb1-f194.google.com [209.85.219.194]) by imf46.hostedemail.com (Postfix) with ESMTP for ; Sat, 3 Oct 2020 21:30:45 +0000 (UTC) Received: by mail-yb1-f194.google.com with SMTP id c3so3751893ybl.0 for ; Sat, 03 Oct 2020 14:30:45 -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=lcSuAPDuR5owltRu+RJj4igLRMjBT0ActUXd/IKaJVQ=; b=sOuG2fbgXWfn3Mdv9IcMc1tTr1NqTg1KIp2+mcpJ/MA6BFTsEEPWIEcUOI2FC1zaFM NW14U7tBeUn9/qIGFv3q9a+IgUFOldU/eonjBOIgTAgFEtMNT45AkFfvXk07tRgnSUPP dsN8ehmwf2swWbW8HcdBJpLyG1Nye/vFZcTXwaO5vFC2VI6Q7xEp7KqQ/NU3PCoXp+ej VGmBAxNwXzWlw6dEe/53ECKc8eVu9qN31KHVAKy8iHy+s0lb4wfCfUPrDKKjZMQPmPpE Z7dDs6JS6u7BQqUMT8vgdDlYKOINy9e11yTt2W0aSEkEdwPq44yXIuPpeyxOIGMx+9HY qS+Q== 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=lcSuAPDuR5owltRu+RJj4igLRMjBT0ActUXd/IKaJVQ=; b=lgJjbHxKgnOJv2+2LD8xaZ2XFeMO0EGWub9QUfnh5FdhufKhZBJL4i+raQmMHxO82L DFF2c02jpJSYkRL0OLT1eAYpggQ6i7haLkmyNSy3fNkqmceWdZYGLFN57fS6RudmravY IqtdeTt6vs8KOgOdJCFA5FpRzu/rDJ9FzJ1wTAyeXyv9DH9JC5ZYAQ+XfsEyKPrLScFY RH3esKphzYwJjjPXMRE5mSp98HInuD3WFXVhNKHP62bbXqS6tcaf8tR+BhKBQeVkrykz 2Ss+Ae5VyyuhA25uT4zYueUdmclkkX8GOcJEpLeLoDkhIqzGnT3tSZv18cHRgPPPfq5P ngVQ== X-Gm-Message-State: AOAM530goC8RrJH2ZRM1azHKOoYgxhBae6jUk6qO11lJq9r7Y6hYBc45 zWtT1Qvi3gKO6FiOCkwc+KcRtkxAaRfpTblYUCNKgw== X-Google-Smtp-Source: ABdhPJyufCi5mVUm8hjg3iNyu72VIpcgco1LGvM7+rVdd/9XqdlFjhzs0SHB5b06m3cdSxM1kiFRG3XHp1Al9pFAv9Y= X-Received: by 2002:a25:14d5:: with SMTP id 204mr10464626ybu.162.1601760641501; Sat, 03 Oct 2020 14:30:41 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Michel Lespinasse Date: Sat, 3 Oct 2020 14:30:28 -0700 Message-ID: Subject: Re: [PATCH 1/2] mmap locking API: Order lock of nascent mm outside lock of live mm To: Jann Horn Cc: Andrew Morton , linux-mm , LKML , "Eric W . Biederman" , Mauro Carvalho Chehab , Sakari Ailus , Jeff Dike , Richard Weinberger , Anton Ivanov , linux-um@lists.infradead.org, Jason Gunthorpe , John Hubbard 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 Fri, Oct 2, 2020 at 9:33 AM Jann Horn wrote: > On Fri, Oct 2, 2020 at 11:18 AM Michel Lespinasse wrote: > > On Thu, Oct 1, 2020 at 6:25 PM Jann Horn wrote: > > > Until now, the mmap lock of the nascent mm was ordered inside the mmap lock > > > of the old mm (in dup_mmap() and in UML's activate_mm()). > > > A following patch will change the exec path to very broadly lock the > > > nascent mm, but fine-grained locking should still work at the same time for > > > the new mm. > > > To do this in a way that lockdep is happy about, let's turn around the lock > > > ordering in both places that currently nest the locks. > > > Since SINGLE_DEPTH_NESTING is normally used for the inner nesting layer, > > > make up our own lock subclass MMAP_LOCK_SUBCLASS_NASCENT and use that > > > instead. > > > > > > The added locking calls in exec_mmap() are temporary; the following patch > > > will move the locking out of exec_mmap(). > > > > Thanks for doing this. > > > > This is probably a silly question, but I am not sure exactly where we > > lock the old MM while bprm is creating the new MM ? I am guessing this > > would be only in setup_arg_pages(), copying the args and environment > > from the old the the new MM ? If that is correct, then wouldn't it be > > sufficient to use mmap_write_lock_nested in setup_arg_pages() ? Or, is > > the issue that we'd prefer to have a killable version of it there ? > > We're also implicitly locking the old MM anytime we take page faults > before exec_mmap(), which basically means the various userspace memory > accesses in do_execveat_common(). This happens after bprm_mm_init(), > so we've already set bprm->vma at that point. Ah yes, I see the issue now. It would be much nicer if copy_strings could coax copy_from_user into taking a nested lock, but of course there is no way to do that. I'm not sure if it'd be reasonable to kmap the source pages like we do for the destination pages ? Adding a nascent lock instead of a nested lock, as you propose, seems to work, but it also looks quite unusual. Not that I have anything better to propose at this point though... Unrelated to the above: copy_from_user and copy_to_user should not be called with mmap_lock held; it may be worth adding these assertions too (probably in separate patches) ? > Uuugh, dammit, I see what happened. Sorry about the trouble. Thanks > for telling me, guess I'll go back to sending patches the way I did it > before. :/ Yeah, I've hit such issues with gmail before too :/ -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies.