Al Viro wrote: > BTW, what your current code does if you have a file bound on another > file, open it, umount -l it, let the dust settle and then do some operation > that triggers tomoyo_get_absolute_path() on it? Because you'll be getting > a vfsmount/dentry pair that has > * dentry == vfsmount->mnt_root > * vfsmount->mnt_parent == vfsmount > * dentry->d_inode being a non-directory > and there is nothing whatsoever in what remains of the pathname. Not a single > component. IOW, you'll get "/" in buf. Might be good in a testsuite - is > there any code in security/tomoyo that would be relying on assumption that > only directory might have a name entirely without components? TOMOYO assumes that only directory ends with '/'. I wrote a test program and compared among below cases. current code current code + your patch current code + your patch + my patch (attached at the bottom) ---------- test1.c ---------- #include <stdio.h> #include <stdlib.h> #include <unistd.h> #include <sys/ioctl.h> #include <sys/mount.h> #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> #define MNT_DETACH 2 static void die(const char *msg) { fprintf(stderr, "%s\n", msg); exit(1); } int main(int argc, char *argv[]) { int fd; if (mount("/tmp/", "/tmp", "tmpfs", 0, NULL)) die("Can't mount"); fd = open("/tmp/file1", O_WRONLY | O_CREAT, 0600); if (fd == EOF) die("Can't create /tmp/file1"); close(fd); fd = open("/tmp/file2", O_WRONLY | O_CREAT, 0600); if (fd == EOF) die("Can't create /tmp/file2"); close(fd); if (mount("/tmp/file1", "/tmp/file2", NULL, MS_BIND, 0)) die("Can't bind mount"); fd = open("/tmp/file2", O_RDONLY | O_CREAT, 0600); if (fd == EOF) die("Can't open /tmp/file2"); if (umount2("/tmp/file2", MNT_DETACH)) die("Can't unmount 2"); ioctl(fd, 0); close(fd); return 0; } ---------- test1.c ---------- Below are the policy checked by TOMOYO. ---- linux b835c0f4 ---- 0: file create /tmp/file1 0600 1: file create /tmp/file2 0600 2: file ioctl / 0x0 3: file mount /tmp/ /tmp/ tmpfs 0x0 4: file mount /tmp/file1 /tmp/file2 --bind 0x0 5: file read /tmp/file2 6: file unmount /tmp/file2 7: file write /tmp/file1 8: file write /tmp/file2 ioctl() handled but its pathname is wrong. ---- linux b835c0f4 with your patch ---- 0: file create /tmp/file1 0600 1: file create /tmp/file2 0600 2: file mount /tmp/ /tmp/ tmpfs 0x0 3: file mount /tmp/file1 /tmp/file2 --bind 0x0 4: file read /tmp/file2 5: file unmount /tmp/file2 6: file write /tmp/file1 7: file write /tmp/file2 ioctl() failed (i.e. regression) because tomoyo_get_absolute_path() failed. The error message was ERROR: Out of memory at tomoyo_realpath_from_path. ---- linux b835c0f4 with your patch and my patch ---- 0: file create /tmp/file1 0600 1: file create /tmp/file2 0600 2: file ioctl tmpfs:/file1 0x0 3: file mount /tmp/ /tmp/ tmpfs 0x0 4: file mount /tmp/file1 /tmp/file2 --bind 0x0 5: file read /tmp/file2 6: file unmount /tmp/file2 7: file write /tmp/file1 8: file write /tmp/file2 ioctl() handled using pathname returned by tomoyo_get_local_path(). > The real question is what pathname do you _want_ in this situation. Define > that and we'll be able to do something about it; Among above three results, the last one will be the best. [PATCH] TOMOYO: Fix pathname handling of disconnected paths. Current tomoyo_realpath_from_path() implementation returns strange pathname when calculating pathname of a file which belongs to lazy unmounted tree. Use local pathname rather than strange absolute pathname in that case. Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> --- a/security/tomoyo/realpath.c +++ b/security/tomoyo/realpath.c @@ -293,8 +293,16 @@ char *tomoyo_realpath_from_path(struct path *path) pos = tomoyo_get_local_path(path->dentry, buf, buf_len - 1); /* Get absolute name for the rest. */ - else + else { pos = tomoyo_get_absolute_path(path, buf, buf_len - 1); + /* + * Fall back to local name if absolute name is not + * available. + */ + if (pos == ERR_PTR(-EINVAL)) + pos = tomoyo_get_local_path(path->dentry, buf, + buf_len - 1); + } encode: if (IS_ERR(pos)) continue; -- 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