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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 31DA5C433EF for ; Mon, 4 Jul 2022 19:56:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 419E36B0071; Mon, 4 Jul 2022 15:55:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3C9926B0073; Mon, 4 Jul 2022 15:55:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2914F6B0074; Mon, 4 Jul 2022 15:55:59 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 1A9B96B0071 for ; Mon, 4 Jul 2022 15:55:59 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id E29A92043F for ; Mon, 4 Jul 2022 19:55:58 +0000 (UTC) X-FDA: 79650473196.19.626A599 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [62.89.141.173]) by imf12.hostedemail.com (Postfix) with ESMTP id 37BE240012 for ; Mon, 4 Jul 2022 19:55:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=1MlVvLG41NeVJL2nvZkPrUdGWixdJlckw5dTfoQTefk=; b=Lcb8/mafbItE20U9aUkaQKp+3D 4wOzZ+NdnUj4WKsXvYsO53MozliGkbS+4GEcVCbqB9GlKxhJORR2Fx87qXmQRdFbVLjV4PR6JhtcN a/DxIoLzPHXhTgs2UWol5vsO5HWZ+3obtKZSYP6I1OknNsGj7tuyPtN0SNFhGlyWoQPZdS25IM3ze gb6OdYCLOARbZHQlnpqTKOD9cUhs+ugo/UxqgHOxVE5miCRoMel/jTs6k8bqKBUVBai0SzVwCKrhX Wx+YeYUooo723+P4l1ZTs4NczTWV/92sLTs8oZS6lwlVjIAbzCSd0VaQSdG44K9s+iLuMUX7MHEoH R+6thDBA==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.95 #2 (Red Hat Linux)) id 1o8SA4-0087lj-68; Mon, 04 Jul 2022 19:55:12 +0000 Date: Mon, 4 Jul 2022 20:55:12 +0100 From: Al Viro To: Linus Torvalds Cc: Alexander Potapenko , Alexei Starovoitov , Andrew Morton , Andrey Konovalov , Andy Lutomirski , Arnd Bergmann , Borislav Petkov , Christoph Hellwig , Christoph Lameter , David Rientjes , Dmitry Vyukov , Eric Dumazet , Greg Kroah-Hartman , Herbert Xu , Ilya Leoshkevich , Ingo Molnar , Jens Axboe , Joonsoo Kim , Kees Cook , Marco Elver , Mark Rutland , Matthew Wilcox , "Michael S. Tsirkin" , Pekka Enberg , Peter Zijlstra , Petr Mladek , Steven Rostedt , Thomas Gleixner , Vasily Gorbik , Vegard Nossum , Vlastimil Babka , kasan-dev , Linux-MM , linux-arch , Linux Kernel Mailing List , Evgenii Stepanov , Nathan Chancellor , Nick Desaulniers , Segher Boessenkool , Vitaly Buka , linux-toolchains Subject: Re: [PATCH v4 43/45] namei: initialize parameters passed to step_into() Message-ID: References: <20220701142310.2188015-1-glider@google.com> <20220701142310.2188015-44-glider@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1656964558; a=rsa-sha256; cv=none; b=kxcg8PBtDbgSptQ1jACq0pa7z5YXoHANoP7xfF+Qb5TBbSVVxASylxQQu9CMapKGmAsqN7 nxfxI4iWJhovaSRRcd8s+1c1dbRUabUMifaiIoyT/35LvxUbxuVDha1P+LBSrqKR0vFuXx dWTyk2awMvXFvPYEG0yOGVDWerSRQlM= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=linux.org.uk header.s=zeniv-20220401 header.b="Lcb8/maf"; dmarc=pass (policy=none) header.from=zeniv.linux.org.uk; spf=none (imf12.hostedemail.com: domain of viro@ftp.linux.org.uk has no SPF policy when checking 62.89.141.173) smtp.mailfrom=viro@ftp.linux.org.uk ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1656964558; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=1MlVvLG41NeVJL2nvZkPrUdGWixdJlckw5dTfoQTefk=; b=8WuABUzJaZ0XTA0A1NkPXbqDKxmen/A8lnLEJP6AE4LEVjWcWbnCkFYgCVWE4cljCiAf/E zB6S9YyhhxXa1BsUnG8+BlZRNqBM3M5ct9LBnl2OuokJDpPR9ji95MOmI18wq19dxmXVT0 irNXyS3TYnzhPnZeuqgjL4CjGcdocuk= X-Rspam-User: X-Rspamd-Server: rspam07 Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=linux.org.uk header.s=zeniv-20220401 header.b="Lcb8/maf"; dmarc=pass (policy=none) header.from=zeniv.linux.org.uk; spf=none (imf12.hostedemail.com: domain of viro@ftp.linux.org.uk has no SPF policy when checking 62.89.141.173) smtp.mailfrom=viro@ftp.linux.org.uk X-Stat-Signature: 95jgb6oi3xeqi9aafk3oaxrfy8f1ysrz X-Rspamd-Queue-Id: 37BE240012 X-HE-Tag: 1656964558-329941 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, Jul 04, 2022 at 12:16:24PM -0700, Linus Torvalds wrote: > On Mon, Jul 4, 2022 at 12:03 PM Al Viro wrote: > > > > Anyway, I've thrown a mount_lock check in there, running xfstests to > > see how it goes... > > So my reaction had been that it would be good to just do something like this: > > diff --git a/fs/namei.c b/fs/namei.c > index 1f28d3f463c3..25c4bcc91142 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -1493,11 +1493,18 @@ static bool __follow_mount_rcu(struct n... > if (flags & DCACHE_MOUNTED) { > struct mount *mounted = __lookup_mnt(path->mnt, dentry); > if (mounted) { > + struct dentry *old_dentry = dentry; > + unsigned old_seq = *seqp; > + > path->mnt = &mounted->mnt; > dentry = path->dentry = mounted->mnt.mnt_root; > nd->state |= ND_JUMPED; > *seqp = read_seqcount_begin(&dentry->d_seq); > *inode = dentry->d_inode; > + > + if (read_seqcount_retry(&old_dentry->d_seq, old_seq)) > + return false; > + > /* > * We don't need to re-check ->d_seq after this > * ->d_inode read - there will be an RCU delay > > but the above is just whitespace-damaged random monkey-scribbling by > yours truly. > > More like a "shouldn't we do something like this" than a serious > patch, in other words. > > IOW, it has *NOT* had a lot of real thought behind it. Purely a > "shouldn't we always clearly check the old sequence number after we've > picked up the new one?" You are checking the wrong thing here. It's really about mount_lock - ->d_seq is *not* bumped when we or attach in some namespace. If there's a mismatch, RCU pathwalk is doomed anyway (it'll fail any form of unlazy) and we might as well bugger off. If it *does* match, we know that both mountpoint and root had been pinned since before the pathwalk, remain pinned as of that check and had a mount connecting them all along. IOW, if we could have arrived to this dentry at any point, we would have gotten that dentry as the next step. We sample into nd->m_seq in path_init() and we want it to stay unchanged all along. If it does, all mountpoints and roots we observe are pinned and their association with each other is stable. It's not dentry -> dentry, it's dentry -> mount -> dentry. The following would've been safe: find mountpoint sample ->d_seq verify whatever had lead us to mountpoint sample mount_lock find mount verify mountpoint's ->d_seq find root of mounted sample its ->d_seq verify mount_lock Correct? Now, note that the last step done against the value we'd sampled in path_init() guarantees that mount hash had not changed through all of that. Which is to say, we can pretend that we'd found mount before ->d_seq of mountpoint might've changed, leaving us with find mountpoint sample ->d_seq verify whatever had lead us to mountpoint find mount find root of mounted sample its ->d_seq verify mount_lock