Jeff Layton <jlayton@xxxxxxxxxx> writes: > There are a few situations where a lookup can end up returning a dentry > without revalidating it, and without checking whether the calling > process has permissions to access it. Two situations identified so far > are: > > 1) LAST_BIND symlinks (such as those under /proc/<pid>) > > 2) file bind mounts > > This patchset is intended to fix this by forcing revalidation of the > returned dentries at appropriate locations. > > In the case of LAST_BIND symlinks it also adds a check to verify that > the target of the symlink is accessible by the current process by > walking mounts and dentries back up to the root and checking permission > on each inode. > > This set fixes the reproducers I have (including the reproducer that > Pavel provided for the permissions bypass). It's still pretty rough > though and I expect that it'll need revision. At this point, I'm mainly > looking to get these questions answered: > > 1) what should we do if these dentries are found to be invalid? Is it ok > to d_invalidate them? Or is that likely to break something (particularly > in the case of file bind mounts)? The normal sequence in do_revalidate should be safe. In practice what we should see is d_drop(). If we access the dentries via another path today we already go through d_revalidate. It is only the reference count on the dentry that keeps them alive and working. The cases I have looked at for distributed filesystems have to call d_drop themselves so I don't know if it would add anything if the vfs called d_revalidate. Especially since FS_REVAL_DOT doesn't have that logic. > 2) I'm using FS_REVAL_DOT as an indicator of whether to force a > d_revalidate. I think that it's appropriate to key off of that flag, but > we may want to rename it (maybe FS_FORCE_D_REVAL ?). Perhaps FS_ALWAYS_REVAL. I don't think it makes much of a difference either way. I expect a rename should come after we fix nfsv4 so there is a chance at pushing the fixes back to stable. > 3) is check_path_accessible racy? It seems to work, but something > doesn't seem quite right with this approach. Is this defeatable somehow? > Could a rename of one of the intermediate path components cause > problems? check_path_accessible seems pretty horrible. If a process is running inside of a subdirectory it doesn't have permissions to access, say a chroot, /proc/self/fd/XXX becomes completely unusable. Eric -- 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
