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 6A537C43334 for ; Sun, 3 Jul 2022 04:00:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9012E6B0072; Sun, 3 Jul 2022 00:00:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 888546B0073; Sun, 3 Jul 2022 00:00:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7025F6B0074; Sun, 3 Jul 2022 00:00:53 -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 5998F6B0072 for ; Sun, 3 Jul 2022 00:00:53 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 2A9A834FB2 for ; Sun, 3 Jul 2022 04:00:53 +0000 (UTC) X-FDA: 79644437586.04.8DF1D61 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [62.89.141.173]) by imf17.hostedemail.com (Postfix) with ESMTP id AA7C04004E for ; Sun, 3 Jul 2022 04:00:52 +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=LQzNJSrqoKMb8FMCvc/7lp2Km4cof1d6Hftc45fNj1g=; b=SlO+fKNyilPz+TVpOIzdYjMW8I Hsqq6l0JUqmKZnzFIfvzTAUxDFekbW1Xjbq8aFbgAuxIsNiVPE0oNLXIPm359WVbU9Seg97M9+O7r OojR6bi66spE6YIUgfO2Wu6DSNRFi+RBxtuotkAqiPYTj9usTSh8wTRcfL43lp3ajxi3TLHEA/12n Y/F8O4V669jsNhJbuRbb9bPkk4FAB58VW1SMi3Rp9ILhYuca0CNGJvhv3GHuBQzPvPNX4V4l0rn2N TJHAKEHb9PbmaIPNcrTdaKaLlrLK2KL2N2ui7v/i5dQvTkEUTppJSfvwBi0BvCcWXWFL7X/ISPBy0 SdFr/OuA==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.95 #2 (Red Hat Linux)) id 1o7qlz-007YYP-Mg; Sun, 03 Jul 2022 03:59:51 +0000 Date: Sun, 3 Jul 2022 04:59:51 +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-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1656820852; 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=LQzNJSrqoKMb8FMCvc/7lp2Km4cof1d6Hftc45fNj1g=; b=eHyGOXhFv/E5t7OWfpSvz5aujjSMGhKtEfGuEZKmzDMmCIPbQthrYrcFjSZo7wOj2VQ0lN Qke+JrXcEn+P8cRmFJc/cDumfooBUreITjECEkEXXWTZxEiRiol1KAIgA7866uKQkvcZJJ kqRb60HQfHIjdn1hc3RB2x1lA+aoVRM= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1656820852; a=rsa-sha256; cv=none; b=ySvHI1Jc1cgroaebUXIROE2aQYb/hvsB3qkdzwjg2JnIbYnXBzrbvndKWvVJAmyxzkT2tg jiC87dyPut96U/wulr5XtDrbKIFtg1h0XhBgU9iJ8WyZU7cBmNECGIfRrJR/q2inErY1IN cGVnpO2gDQv/rl+6JRAAYsb2XC7U+hE= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=linux.org.uk header.s=zeniv-20220401 header.b=SlO+fKNy; dmarc=pass (policy=none) header.from=zeniv.linux.org.uk; spf=none (imf17.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: z63ic5uy71ezhp7xu5ysmuwaae1753wa X-Rspam-User: Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=linux.org.uk header.s=zeniv-20220401 header.b=SlO+fKNy; dmarc=pass (policy=none) header.from=zeniv.linux.org.uk; spf=none (imf17.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-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: AA7C04004E X-HE-Tag: 1656820852-275381 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 Sat, Jul 02, 2022 at 10:23:16AM -0700, Linus Torvalds wrote: > On Fri, Jul 1, 2022 at 7:25 AM Alexander Potapenko wrote: > > > > Under certain circumstances initialization of `unsigned seq` and > > `struct inode *inode` passed into step_into() may be skipped. > > In particular, if the call to lookup_fast() in walk_component() > > returns NULL, and lookup_slow() returns a valid dentry, then the > > `seq` and `inode` will remain uninitialized until the call to > > step_into() (see [1] for more info). > So while I think this needs to be fixed, I think I'd really prefer to > make the initialization and/or usage rules stricter or at least > clearer. Disclaimer: the bits below are nowhere near what I consider a decent explanation; this might serve as the first approximation, but I really need to get some sleep before I get it into coherent shape. 4 hours of sleep today... The rules are * no pathname resolution without successful path_init(). IOW, path_init() failure is an instant fuck off. * path_init() success sets nd->inode. In all cases. * nd->inode must be set - LOOKUP_RCU or not, we simply cannot proceed without it. * in non-RCU mode nd->inode must be equal to nd->path.dentry->d_inode. * in RCU mode nd->inode must be equal to a value observed in nd->path.dentry->d_inode while nd->path.dentry->d_seq had been equal to nd->seq. * step_into() gets a dentry/inode/seq triple. In non-RCU mode inode and seq are ignored; in RCU mode they must satisfy the same relationship we have for nd->path.dentry/nd->inode/nd->seq. > Of course, sometimes the "only get used for LOOKUP_RCU" is very very > unclear, because even without being an RCU lookup, step_into() will > save it into nd->inode/seq. So the values were "used", and > initializing them makes them valid, but then *that* copy must not then > be used unless RCU was set. You are misreading that (and I admit that it badly needs documentation). The whole point of step_into() is to move over to new place. nd->inode *MUST* be set on success, no matter what. > - I look at that follow_dotdot*() caller case, and think "that looks > very similar to the lookup_fast() case, but then we have *very* > different initialization rules". follow_dotdot() might as well lose inodep and seqp arguments - everything would've worked just as well without those. We would've gotten the same complaints about uninitialized values passed to step_into(), though. This if (unlikely(!parent)) error = step_into(nd, WALK_NOFOLLOW, nd->path.dentry, nd->inode, nd->seq); in handle_dots() probably contributes to confusion - it's the "we have stepped on .. in the root, just jump into whatever's mounted on it" case. In non-RCU case it looks like a use of nd->seq in non-RCU mode; however, in that case step_into() will end up ignoring the last two arguments. I'll post something more coherent after I get some sleep. Sorry... ;-/