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