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=-0.8 required=3.0 tests=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 480A3C4360C for ; Sat, 12 Oct 2019 04:18:08 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D75232067B for ; Sat, 12 Oct 2019 04:18:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D75232067B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=cyphar.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 46qs4X0FLbzDqfJ for ; Sat, 12 Oct 2019 15:18:04 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=cyphar.com (client-ip=80.241.60.219; helo=mx2a.mailbox.org; envelope-from=cyphar@cyphar.com; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=cyphar.com Received: from mx2a.mailbox.org (mx2a.mailbox.org [80.241.60.219]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 46qs2P1FPCzDqcV for ; Sat, 12 Oct 2019 15:16:13 +1100 (AEDT) Received: from smtp1.mailbox.org (smtp1.mailbox.org [IPv6:2001:67c:2050:105:465:1:1:0]) (using TLSv1.2 with cipher ECDHE-RSA-CHACHA20-POLY1305 (256/256 bits)) (No client certificate requested) by mx2a.mailbox.org (Postfix) with ESMTPS id A095FA18A9; Sat, 12 Oct 2019 06:16:08 +0200 (CEST) X-Virus-Scanned: amavisd-new at heinlein-support.de Received: from smtp1.mailbox.org ([80.241.60.240]) by gerste.heinlein-support.de (gerste.heinlein-support.de [91.198.250.173]) (amavisd-new, port 10030) with ESMTP id XP4SfIU7eOyJ; Sat, 12 Oct 2019 06:16:03 +0200 (CEST) Date: Sat, 12 Oct 2019 15:15:41 +1100 From: Aleksa Sarai To: Linus Torvalds Subject: Re: [PATCH v14 2/6] namei: LOOKUP_IN_ROOT: chroot-like path resolution Message-ID: <20191012041541.milbmfbjpj5bcl5a@yavin.dot.cyphar.com> References: <20191010054140.8483-1-cyphar@cyphar.com> <20191010054140.8483-3-cyphar@cyphar.com> <20191012040815.gnc43cfmo5mnv67u@yavin.dot.cyphar.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="2tj3n7ljtot3r57d" Content-Disposition: inline In-Reply-To: <20191012040815.gnc43cfmo5mnv67u@yavin.dot.cyphar.com> X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-ia64@vger.kernel.org, Linux-sh list , Peter Zijlstra , Rasmus Villemoes , Alexei Starovoitov , Linux Kernel Mailing List , David Howells , "open list:KERNEL SELFTEST FRAMEWORK" , sparclinux@vger.kernel.org, Jiri Olsa , linux-arch , linux-s390 , Tycho Andersen , Aleksa Sarai , Shuah Khan , Alexander Shishkin , Ingo Molnar , Linux ARM , linux-mips@vger.kernel.org, linux-xtensa@linux-xtensa.org, Kees Cook , Arnd Bergmann , Jann Horn , linux-m68k , Al Viro , Andy Lutomirski , Shuah Khan , Namhyung Kim , David Drysdale , Christian Brauner , "J. Bruce Fields" , GNU C Library , linux-parisc@vger.kernel.org, Linux API , Chanho Min , Jeff Layton , Oleg Nesterov , Eric Biederman , alpha , linux-fsdevel , Andrew Morton , linuxppc-dev@lists.ozlabs.org, Linux Containers Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" --2tj3n7ljtot3r57d Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 2019-10-12, Aleksa Sarai wrote: > On 2019-10-10, Linus Torvalds wrote: > > On Wed, Oct 9, 2019 at 10:42 PM Aleksa Sarai wrote: > > > > > > --- a/fs/namei.c > > > +++ b/fs/namei.c > > > @@ -2277,6 +2277,11 @@ static const char *path_init(struct nameidata = *nd, unsigned flags) > > > > > > nd->m_seq =3D read_seqbegin(&mount_lock); > > > > > > + /* LOOKUP_IN_ROOT treats absolute paths as being relative-to-= dirfd. */ > > > + if (flags & LOOKUP_IN_ROOT) > > > + while (*s =3D=3D '/') > > > + s++; > > > + > > > /* Figure out the starting path and root (if needed). */ > > > if (*s =3D=3D '/') { > > > error =3D nd_jump_root(nd); > >=20 > > Hmm. Wouldn't this make more sense all inside the if (*s =3D- '/') test? > > That way if would be where we check for "should we start at the root", > > which seems to make more sense conceptually. >=20 > I don't really agree (though I do think that both options are pretty > ugly). Doing it before the block makes it clear that absolute paths are > just treated relative-to-dirfd -- doing it inside the block makes it > look more like "/" is a special-case for nd_jump_root(). And while that Sorry, I meant "special-case for LOOKUP_IN_ROOT". > is somewhat true, this is just a side-effect of making the code more > clean -- my earlier versions reworked the dirfd handling to always grab > nd->root first if LOOKUP_IS_SCOPED. I switched to this method based on > Al's review. >=20 > In fairness, I do agree that the lonely while loop looks ugly. And with the old way I did it (where we grabbed nd->root first) the semantics were slightly more clear -- stripping leading "/"s doesn't really look as "clearly obvious" as grabbing nd->root beforehand and treating "/"s normally. But the code was also needlessly more complex. > > That test for '/' currently has a "} else if (..)", but that's > > pointless since it ends with a "return" anyway. So the "else" logic is > > just noise. >=20 > This depends on the fact that LOOKUP_BENEATH always triggers -EXDEV for > nd_jump_root() -- if we ever add another "scoped lookup" flag then the > logic will have to be further reworked. >=20 > (It should be noted that the new version doesn't always end with a > "return", but you could change it to act that way given the above > assumption.) >=20 > > And if you get rid of the unnecessary else, moving the LOOKUP_IN_ROOT > > inside the if-statement works fine. > >=20 > > So this could be something like > >=20 > > --- a/fs/namei.c > > +++ b/fs/namei.c > > @@ -2194,11 +2196,19 @@ static const char *path_init(struct > > nameidata *nd, unsigned flags) > >=20 > > nd->m_seq =3D read_seqbegin(&mount_lock); > > if (*s =3D=3D '/') { > > - set_root(nd); > > - if (likely(!nd_jump_root(nd))) > > - return s; > > - return ERR_PTR(-ECHILD); > > - } else if (nd->dfd =3D=3D AT_FDCWD) { > > + /* LOOKUP_IN_ROOT treats absolute paths as being > > relative-to-dirfd. */ > > + if (!(flags & LOOKUP_IN_ROOT)) { > > + set_root(nd); > > + if (likely(!nd_jump_root(nd))) > > + return s; > > + return ERR_PTR(-ECHILD); > > + } > > + > > + /* Skip initial '/' for LOOKUP_IN_ROOT */ > > + do { s++; } while (*s =3D=3D '/'); > > + } > > + > > + if (nd->dfd =3D=3D AT_FDCWD) { > > if (flags & LOOKUP_RCU) { > > struct fs_struct *fs =3D current->fs; > > unsigned seq; > >=20 > > instead. The patch ends up slightly bigger (due to the re-indentation) > > but now it handles all the "start at root" in the same place. Doesn't > > that make sense? >=20 > It is correct (though I'd need to clean it up a bit to handle > nd_jump_root() correctly), and if you really would like me to change it > I will -- but I just don't agree that it's cleaner. --=20 Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH --2tj3n7ljtot3r57d Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQSxZm6dtfE8gxLLfYqdlLljIbnQEgUCXaFTZwAKCRCdlLljIbnQ EsoyAQDTzuZiYFf1pW7tBymiBU+fSwe8/adLefrS9DH9oll0LQD+M2OnaEH8wDH5 iC8HumWtmk7eUf1CcqNfwVk4mwQXYwU= =dEoe -----END PGP SIGNATURE----- --2tj3n7ljtot3r57d--