AV: > LOOKUP_FOLLOW is a separate story - the issue here is that you are
> exposing internal details of the pathname resolution to the userland
> and that's essentially a demand to keep them stable from now on; however,
> I'm seriously at loss as to _what_ to keep stable.
> "This call of ->lookup() gets LOOKUP_FOLLOW in flags" doesn't map
> to any natural semantics I can think of; it might accidentally happen
> to match something useful, but I don't see what it is
Hard to argue with that.
AV: > Your userland code does something rather odd with [LOOKUP_FOLLOW],
> but it's buried inside your state machine and I don't understand it
> well enough to tell what's going on.
Yeah, I've gotten lost trying to follow the code off into the state machines
more than once <g>... so I talked to Walt about it. If I can be allowed
to wave my hands about the state machine, I understand that
PVFS2_LOOKUP_LINK_NO_FOLLOW and PVFS2_LOOKUP_LINK_FOLLOW are used
in optimizing lookups, outside the scope of the VFS, that involve symlinks.
The kernel module, though, looks up one component of a path at a time.
LOOKUP_FOLLOW is commented in Documentation/filesystems/path-lookup.md.
./src/client/sysint/sys-lookup.c and ./src/client/sysint/sys-lookup.sm
has some code and comments that might help show what the original developers
were thinking when they decided to use use LOOKUP_FOLLOW in the kernel module:
/*
check if we need to skip the symlink resolution
(in the case that it's the final segment and the
user requested that we do NOT follow the last link)
*/
AV: > As the result, having your filesystem mounted inside a chroot
> jail would open a nice way for non-priveleged process to break out -
> just create an absolute symlink somewhere on that filesystem and go
> through it.
I made some chroot tests without being able to break out, I would show
you the results if wanted... but still... we are misusing LOOKUP_FOLLOW.
So... I talked with Walt about changing...
new_op->upcall.req.lookup.sym_follow = flags & LOOKUP_FOLLOW;
to this...
new_op->upcall.req.lookup.sym_follow = PVFS2_LOOKUP_LINK_NO_FOLLOW;
I made the change and did some tests...
Not only does the change not break anything, it fixes something that
we didn't know was broken!
I mount Orangefs at /pvfsmnt...
$ ls /pvfsmnt
dir1
dir3
$ ls -l /pvfsmnt/dir3/dir2.file.symlink
lrwxrwxrwx. 1 root root 17 Dec 3 10:22
/pvfsmnt/dir3/dir2.file.symlink -> ../dir1/dir2/file
$ find /pvfsmnt/dir1
/pvfsmnt/dir1
/pvfsmnt/dir1/dir2
/pvfsmnt/dir1/dir2/file
Right after a reboot, when there's pretty much nothing in the dcache...
$ ls /pvfsmnt/dir3/dir2.file.symlink
ls: cannot access /pvfsmnt/dir3/dir2.file.symlink: Invalid argument
$ find /pvfsmnt
.
.
.
$ ls /pvfsmnt/dir3/dir2.file.symlink
/pvfsmnt/dir3/dir2.file.symlink
The failed ls of dir2.file.symlink gets a "Error code 1610612744: Path contains
non-PVFS elements" from the orangefs server...
When lookups can be done through the dcache, the kernel module follows a
different code path that always uses PVFS2_LOOKUP_LINK_NO_FOLLOW.
After the change, there is no initial failure - since now both code paths
use PVFS2_LOOKUP_LINK_NO_FOLLOW.
So... Al noticed that our use of LOOKUP_FOLLOW was bogus, I changed
it to something that I think is reasonable, and now stuff works better.
I know Linus hates fixes that involve thrashing around until stuff seems
to work right... I hope this fix is "on purpose" enough...
-Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
