[RFC] fix d_absolute_path() interplay with fsmount()
diff mbox series

Message ID 20190822035134.GK1131@ZenIV.linux.org.uk
State New
Headers show
Series
  • [RFC] fix d_absolute_path() interplay with fsmount()
Related show

Commit Message

Al Viro Aug. 22, 2019, 3:51 a.m. UTC
[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


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.

Comments

Tetsuo Handa Aug. 30, 2019, 10:11 a.m. UTC | #1
On 2019/08/22 12:51, Al Viro wrote:
> [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".

I think you can send this patch to linux-next.

Patch
diff mbox series

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;