From 094905c87771b0682610f9d6d92c921180a0ecda Mon Sep 17 00:00:00 2001 From: Daniel Rosenberg Date: Mon, 11 Jul 2022 15:05:15 -0700 Subject: [PATCH] ANDROID: fuse-bpf: Always call revalidate for backing If we have a backing dentry, we must call it's revalidate always, or we may end up using an invalid lower dentry. Revalidate is called for dentries, not inodes, and the dentry may be negative. This goes against all of the macro conventions, so we're just calling the backing function directly. Signed-off-by: Daniel Rosenberg Bug: 219958836 Test: fuse_test Change-Id: Ia28da5bd2ce42b40466c577137d5440d9f2f6600 --- fs/fuse/backing.c | 9 +- fs/fuse/dir.c | 108 ++++++++++++------ fs/fuse/fuse_i.h | 5 +- .../selftests/filesystems/fuse/fuse_test.c | 21 ++++ 4 files changed, 97 insertions(+), 46 deletions(-) diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c index db71e77f2bfb..b536fd9e99b7 100644 --- a/fs/fuse/backing.c +++ b/fs/fuse/backing.c @@ -1306,8 +1306,7 @@ struct dentry *fuse_lookup_finalize(struct fuse_bpf_args *fa, struct inode *dir, return d_splice_alias(inode, entry); } -int fuse_revalidate_backing(struct fuse_bpf_args *fa, struct inode *dir, - struct dentry *entry, unsigned int flags) +int fuse_revalidate_backing(struct dentry *entry, unsigned int flags) { struct fuse_dentry *fuse_dentry = get_fuse_dentry(entry); struct dentry *backing_entry = fuse_dentry->backing_path.dentry; @@ -1324,12 +1323,6 @@ int fuse_revalidate_backing(struct fuse_bpf_args *fa, struct inode *dir, return 1; } -void *fuse_revalidate_finalize(struct fuse_bpf_args *fa, struct inode *dir, - struct dentry *entry, unsigned int flags) -{ - return 0; -} - int fuse_canonical_path_initialize(struct fuse_bpf_args *fa, struct fuse_dummy_io *fdi, const struct path *path, diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index e86768ef91df..0eacbe8cf9c8 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -173,6 +173,44 @@ static void fuse_lookup_init(struct fuse_conn *fc, struct fuse_args *args, args->out_args[1].value = bpf_outarg; } +#ifdef CONFIG_FUSE_BPF +static bool backing_data_changed(struct fuse_inode *fi, struct dentry *entry, + struct fuse_entry_bpf *bpf_arg) +{ + struct path new_backing_path; + struct inode *new_backing_inode; + struct bpf_prog *bpf = NULL; + int err; + bool ret = true; + + if (!entry) + return false; + + get_fuse_backing_path(entry, &new_backing_path); + new_backing_inode = fi->backing_inode; + ihold(new_backing_inode); + + err = fuse_handle_backing(bpf_arg, &new_backing_inode, &new_backing_path); + + if (err) + goto put_inode; + + err = fuse_handle_bpf_prog(bpf_arg, entry->d_parent->d_inode, &bpf); + if (err) + goto put_bpf; + + ret = (bpf != fi->bpf || fi->backing_inode != new_backing_inode || + !path_equal(&get_fuse_dentry(entry)->backing_path, &new_backing_path)); +put_bpf: + if (bpf) + bpf_prog_put(bpf); +put_inode: + iput(new_backing_inode); + path_put(&new_backing_path); + return ret; +} +#endif + /* * Check whether the dentry is still valid * @@ -193,7 +231,28 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags) inode = d_inode_rcu(entry); if (inode && fuse_is_bad(inode)) goto invalid; - else if (time_before64(fuse_dentry_time(entry), get_jiffies_64()) || + +#ifdef CONFIG_FUSE_BPF + /* TODO: Do we need bpf support for revalidate? + * If the lower filesystem says the entry is invalid, FUSE probably shouldn't + * try to fix that without going through the normal lookup path... + */ + if (get_fuse_dentry(entry)->backing_path.dentry) { + ret = fuse_revalidate_backing(entry, flags); + if (ret <= 0) { + goto out; + } + } + /* TODO: Respect timeouts for lookups with backing inodes */ + parent = dget_parent(entry); + if (get_fuse_inode(d_inode_rcu(parent))->backing_inode) { + dput(parent); + ret = 1; + goto out; + } + dput(parent); +#endif + if (time_before64(fuse_dentry_time(entry), get_jiffies_64()) || (flags & LOOKUP_REVAL)) { struct fuse_entry_out outarg; struct fuse_entry_bpf bpf_arg; @@ -208,20 +267,6 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags) ret = -ECHILD; if (flags & LOOKUP_RCU) goto out; -#ifdef CONFIG_FUSE_BPF - { - struct fuse_err_ret fer; - - fer = fuse_bpf_backing(entry->d_parent->d_inode, - struct fuse_lookup_io, - fuse_lookup_initialize, - fuse_revalidate_backing, - fuse_revalidate_finalize, - d_inode(entry->d_parent), entry, flags); - if (fer.ret) - return PTR_ERR(fer.result); - } -#endif fm = get_fuse_mount(inode); forget = fuse_alloc_forget(); @@ -233,34 +278,29 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags) parent = dget_parent(entry); + /* TODO: Once we're handling timeouts for backing inodes, do a + * bpf based lookup_revalidate here. + */ + if (get_fuse_inode(parent->d_inode)->backing_inode) { + ret = 1; + goto out; + } + fuse_lookup_init(fm->fc, &args, get_node_id(d_inode(parent)), &entry->d_name, &outarg, &bpf_arg.out); ret = fuse_simple_request(fm, &args); - -#ifdef CONFIG_FUSE_BPF - if (ret == sizeof(bpf_arg.out)) { - ret = -ENOENT; - if (!entry) - goto out; - - ret = fuse_handle_backing(&bpf_arg, &get_fuse_inode(inode)->backing_inode, - &get_fuse_dentry(entry)->backing_path); - if (ret) - goto out; - - ret = fuse_handle_bpf_prog(&bpf_arg, parent->d_inode, - &get_fuse_inode(inode)->bpf); - if (ret) - goto out; - } -#endif dput(parent); + /* Zero nodeid is same as -ENOENT */ if (!ret && !outarg.nodeid) ret = -ENOENT; - if (!ret) { + if (!ret || ret == sizeof(bpf_arg.out)) { fi = get_fuse_inode(inode); if (outarg.nodeid != get_node_id(inode) || +#ifdef CONFIG_FUSE_BPF + (ret == sizeof(bpf_arg.out) && + backing_data_changed(fi, entry, &bpf_arg)) || +#endif (bool) IS_AUTOMOUNT(inode) != (bool) (outarg.attr.flags & FUSE_ATTR_SUBMOUNT)) { fuse_queue_forget(fm->fc, forget, outarg.nodeid, 1); diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 163340ef066d..87de62763a6c 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -1608,10 +1608,7 @@ int fuse_lookup_backing(struct fuse_bpf_args *fa, struct inode *dir, struct dentry *entry, unsigned int flags); struct dentry *fuse_lookup_finalize(struct fuse_bpf_args *fa, struct inode *dir, struct dentry *entry, unsigned int flags); -int fuse_revalidate_backing(struct fuse_bpf_args *fa, struct inode *dir, - struct dentry *entry, unsigned int flags); -void *fuse_revalidate_finalize(struct fuse_bpf_args *fa, struct inode *dir, - struct dentry *entry, unsigned int flags); +int fuse_revalidate_backing(struct dentry *entry, unsigned int flags); int fuse_canonical_path_initialize(struct fuse_bpf_args *fa, struct fuse_dummy_io *fdi, diff --git a/tools/testing/selftests/filesystems/fuse/fuse_test.c b/tools/testing/selftests/filesystems/fuse/fuse_test.c index cc47e7f5e3fd..58a0dd587fec 100644 --- a/tools/testing/selftests/filesystems/fuse/fuse_test.c +++ b/tools/testing/selftests/filesystems/fuse/fuse_test.c @@ -1862,6 +1862,27 @@ static int bpf_test_revalidate_handle_backing_fd(const char *mount_dir) })); TESTSYSCALL(close(backing_fd)); // Step 1: Lookup folder1 as a reaction to revalidate call + // This attempts to change the backing node, which is not allowed on revalidate + TESTFUSELOOKUP(folder1, 0); + TESTSYSCALL(s_fuse_attr(s_path(s(ft_src), s(folder1)), &attr)); + TEST(backing_fd = s_open(s_path(s(ft_src), s(folder1)), + O_DIRECTORY | O_RDONLY | O_CLOEXEC), + backing_fd != -1); + TESTFUSEOUT2(fuse_entry_out, ((struct fuse_entry_out) { + .nodeid = attr.ino, + .generation = 0, + .entry_valid = UINT64_MAX, + .attr_valid = UINT64_MAX, + .entry_valid_nsec = UINT32_MAX, + .attr_valid_nsec = UINT32_MAX, + .attr = attr, + }), fuse_entry_bpf_out, ((struct fuse_entry_bpf_out) { + .backing_action = FUSE_ACTION_REPLACE, + .backing_fd = backing_fd, + })); + TESTSYSCALL(close(backing_fd)); + + // Lookup folder1 as a reaction to failed revalidate TESTFUSELOOKUP(folder1, 0); TESTSYSCALL(s_fuse_attr(s_path(s(ft_src), s(folder1)), &attr)); TEST(backing_fd = s_open(s_path(s(ft_src), s(folder1)),