All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: John Johansen <john.johansen@canonical.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	David Howells <dhowells@redhat.com>,
	linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	torvalds@linux-foundation.org,
	linux-security-module@vger.kernel.org
Subject: [RFC][PATCH] fix d_absolute_path() interplay with fsmount()
Date: Thu, 22 Aug 2019 04:51:34 +0100	[thread overview]
Message-ID: <20190822035134.GK1131@ZenIV.linux.org.uk> (raw)
In-Reply-To: <16ae946d-dbbe-9be9-9b22-866b3cd1cd7e@i-love.sakura.ne.jp>

[bringing a private thread back to the lists]

There's a bug in interplay of fsmount() and d_absolute_path().
Namely, the check in d_absolute_path() treats the
not-yet-attached mount as "reached absolute root".
AFAICS, the right fix is this

diff --git a/fs/d_path.c b/fs/d_path.c
index a7d0a96b35ce..0f1fc1743302 100644
--- a/fs/d_path.c
+++ b/fs/d_path.c
@@ -116,8 +116,10 @@ static int prepend_path(const struct path *path,
 				vfsmnt = &mnt->mnt;
 				continue;
 			}
-			if (!error)
-				error = is_mounted(vfsmnt) ? 1 : 2;
+			if (is_mounted(vfsmnt) && !is_anon_ns(mnt->mnt_ns))
+				error = 1;	// absolute root
+			else
+				error = 2;	// detached or not attached yet
 			break;
 		}
 		parent = dentry->d_parent;

but that would slightly change the behaviour in another case.
Namely, nfs4 mount-time temporary namespaces.  There we have
the following: mount -t nfs4 server:/foo/bar/baz /mnt
will
        * set a temporary namespace, matching the mount tree as
exported by server
        * mount the root export there
        * traverse foo/bar/baz in that namespace, triggering
automounts when we cross the filesystem boundaries on server.
        * grab whatever we'd arrived at; that's what we'll
be mounting.
        * dissolve the temp namespace.

If you trigger some LSM hook (e.g. in permission checks on
that pathname traversal) for objects in that temp namespace,
do you want d_absolute_path() to succeed (and give a pathname
relative to server's root export), or should it rather fail?

AFAICS, apparmor and tomoyo are the only things that might
care either way; I would go with "fail, it's not an absolute
path" (and that's what the patch above will end up doing),
but it's really up to you.

It definitely ought to fail for yet-to-be-attached case, though;
it doesn't, and that's a bug that needs to be fixed.  Mea culpa.

  parent reply	other threads:[~2019-08-22  3:51 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-19 17:08 [PATCH 00/10] VFS: Provide new mount UAPI David Howells
2019-02-19 17:08 ` [PATCH 01/10] vfs: syscall: Add open_tree(2) to reference or clone a mount David Howells
2019-02-19 17:08 ` [PATCH 02/10] vfs: syscall: Add move_mount(2) to move mounts around David Howells
2019-02-20 12:32   ` Alan Jenkins
2019-02-20 12:41     ` Alan Jenkins
2019-02-20 16:23   ` Jann Horn
2019-07-08 12:02   ` Tetsuo Handa
2019-07-08 13:18     ` Al Viro
2019-07-08 17:12       ` Eric W. Biederman
2019-07-08 18:01         ` Al Viro
2019-07-08 18:13           ` Al Viro
2019-07-08 20:21           ` Al Viro
2019-07-09  0:13             ` Eric W. Biederman
2019-07-09 10:51               ` Tetsuo Handa
2019-07-22 10:12                 ` Tetsuo Handa
2019-07-23  4:16                   ` John Johansen
2019-07-23 13:45                     ` Tetsuo Handa
2019-08-06 10:43                       ` Tetsuo Handa
2019-08-09 15:44                         ` [PATCH] LSM: Disable move_mount() syscall when TOMOYO or AppArmor is enabled Tetsuo Handa
2019-08-22  3:51                         ` Al Viro [this message]
2019-08-30 10:11                           ` [RFC][PATCH] fix d_absolute_path() interplay with fsmount() Tetsuo Handa
2019-07-23 21:45             ` [PATCH 02/10] vfs: syscall: Add move_mount(2) to move mounts around James Morris
2019-07-23 23:30               ` Al Viro
2019-02-19 17:08 ` [PATCH 03/10] teach move_mount(2) to work with OPEN_TREE_CLONE David Howells
2019-02-20 18:59   ` Alan Jenkins
2019-02-26 17:45   ` Alan Jenkins
2019-02-19 17:08 ` [PATCH 04/10] Make anon_inodes unconditional David Howells
2019-02-19 17:09 ` [PATCH 05/10] vfs: syscall: Add fsopen() to prepare for superblock creation David Howells
2019-02-19 17:09 ` [PATCH 06/10] vfs: Implement logging through fs_context David Howells
2019-02-19 17:09 ` [PATCH 07/10] vfs: syscall: Add fsconfig() for configuring and managing a context David Howells
2019-02-19 17:09 ` [PATCH 08/10] vfs: syscall: Add fsmount() to create a mount for a superblock David Howells
2019-02-19 17:09 ` [PATCH 09/10] vfs: syscall: Add fspick() to select a superblock for reconfiguration David Howells
2019-02-19 17:09 ` [PATCH 10/10] vfs: Add a sample program for the new mount API David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190822035134.GK1131@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=dhowells@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=john.johansen@canonical.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.