From 306c4b88cffebcdf6a69b0b16d79d3c66411c7bd Mon Sep 17 00:00:00 2001 From: Dmitrii Merkurev Date: Fri, 8 Jul 2022 23:38:18 +0000 Subject: [PATCH] UPSTREAM: ANDROID: fuse-bpf: Fix revalidate error path and backing handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we have 2 different problems 1. Every revalidate considered as a error because of added args->out_argvar = true; inside fuse_lookup_init which makes fuse_simple_request return out argument size which is considered as an error by revalidate code. 2. We’re ignoring backing_fd and bpf_program set by daemon lookup code called by revalidate. Problem 1 makes any revalidate (lookup to userspace) useless and any result lead us to the full lookup because it was interpreted as an error. This CL fixes both and introducing revalidate test case which makes sure: 1. We’re receiving only one lookup as a part of revalidate 2. We’re setting backing_fd as a part of revalidate’s lookup result Test is failed before the fix and passed after. Bug: 219958836 Test: Booted device 5 times to make sure we’re not receiving redundant lookups anymore. Test: selftests Signed-off-by: Dmitrii Merkurev Change-Id: Ifa62e56b42ca5580b25682eb5f16b5c91826cf49 --- fs/fuse/backing.c | 177 +++++++++++++++++++++++++++------------------- fs/fuse/dir.c | 67 ++++++++---------- fs/fuse/fuse_i.h | 7 ++ 3 files changed, 141 insertions(+), 110 deletions(-) diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c index 3fea31bec40a..d21ab1a076ec 100644 --- a/fs/fuse/backing.c +++ b/fs/fuse/backing.c @@ -1172,6 +1172,105 @@ int fuse_lookup_backing(struct fuse_bpf_args *fa, struct inode *dir, return 0; } +int handle_inode_backing_fd(struct inode *inode, struct dentry *entry, + struct fuse_entry_bpf_out *febo, + struct fuse_entry_bpf *feb) +{ + struct fuse_inode *fi = get_fuse_inode(inode); + struct fuse_dentry *fd = get_fuse_dentry(entry); + int ret = 0; + + switch (febo->backing_action) { + case FUSE_ACTION_KEEP: + /* backing inode/path are added in fuse_lookup_backing */ + break; + + case FUSE_ACTION_REMOVE: + iput(fi->backing_inode); + fi->backing_inode = NULL; + path_put_init(&fd->backing_path); + break; + + case FUSE_ACTION_REPLACE: { + struct file *backing_file = feb->backing_file; + + if (!backing_file) + return -EINVAL; + if (IS_ERR(backing_file)) + return PTR_ERR(backing_file); + + if (fi->backing_inode) + iput(fi->backing_inode); + fi->backing_inode = backing_file->f_inode; + ihold(fi->backing_inode); + + path_put(&fd->backing_path); + fd->backing_path = backing_file->f_path; + path_get(&fd->backing_path); + + fput(backing_file); + break; + } + + default: + return -EINVAL; + } + + return ret; +} + +int handle_inode_bpf(struct inode *inode, struct inode *parent, + struct fuse_entry_bpf_out *febo, + struct fuse_entry_bpf *feb) +{ + struct fuse_inode *fi = get_fuse_inode(inode); + struct fuse_inode *pi; + int ret = 0; + + // Parent isn't presented, but we want to keep + // Don't touch bpf program at all in this case + if (febo->bpf_action == FUSE_ACTION_KEEP && !parent) { + goto out; + } + + if (fi->bpf) { + bpf_prog_put(fi->bpf); + fi->bpf = NULL; + } + + switch (febo->bpf_action) { + case FUSE_ACTION_KEEP: + pi = get_fuse_inode(parent); + fi->bpf = pi->bpf; + if (fi->bpf) + bpf_prog_inc(fi->bpf); + break; + + case FUSE_ACTION_REMOVE: + break; + + case FUSE_ACTION_REPLACE: { + struct file *bpf_file = feb->bpf_file; + struct bpf_prog *bpf_prog = ERR_PTR(-EINVAL); + + if (bpf_file && !IS_ERR(bpf_file)) + bpf_prog = fuse_get_bpf_prog(bpf_file); + + if (IS_ERR(bpf_prog)) + return PTR_ERR(bpf_prog); + + fi->bpf = bpf_prog; + break; + } + + default: + return -EINVAL; + } + +out: + return ret; +} + struct dentry *fuse_lookup_finalize(struct fuse_bpf_args *fa, struct inode *dir, struct dentry *entry, unsigned int flags) { @@ -1182,6 +1281,7 @@ struct dentry *fuse_lookup_finalize(struct fuse_bpf_args *fa, struct inode *dir, struct fuse_entry_out *feo = fa->out_args[0].value; struct fuse_entry_bpf_out *febo = fa->out_args[1].value; struct fuse_entry_bpf *feb = container_of(febo, struct fuse_entry_bpf, out); + int error = -1; u64 target_nodeid = 0; fd = get_fuse_dentry(entry); @@ -1202,78 +1302,13 @@ struct dentry *fuse_lookup_finalize(struct fuse_bpf_args *fa, struct inode *dir, if (IS_ERR(inode)) return ERR_PTR(PTR_ERR(inode)); - /* TODO Make sure this handles invalid handles */ - /* TODO Do we need the same code in revalidate */ - if (get_fuse_inode(inode)->bpf) { - bpf_prog_put(get_fuse_inode(inode)->bpf); - get_fuse_inode(inode)->bpf = NULL; - } + error = handle_inode_bpf(inode, dir, febo, feb); + if (error) + return ERR_PTR(error); - switch (febo->bpf_action) { - case FUSE_ACTION_KEEP: - get_fuse_inode(inode)->bpf = get_fuse_inode(dir)->bpf; - if (get_fuse_inode(inode)->bpf) - bpf_prog_inc(get_fuse_inode(inode)->bpf); - break; - - case FUSE_ACTION_REMOVE: - get_fuse_inode(inode)->bpf = NULL; - break; - - case FUSE_ACTION_REPLACE: { - struct file *bpf_file = feb->bpf_file; - struct bpf_prog *bpf_prog = ERR_PTR(-EINVAL); - - if (bpf_file && !IS_ERR(bpf_file)) - bpf_prog = fuse_get_bpf_prog(bpf_file); - - if (IS_ERR(bpf_prog)) - return ERR_PTR(PTR_ERR(bpf_prog)); - - get_fuse_inode(inode)->bpf = bpf_prog; - break; - } - - default: - return ERR_PTR(-EIO); - } - - switch (febo->backing_action) { - case FUSE_ACTION_KEEP: - /* backing inode/path are added in fuse_lookup_backing */ - break; - - case FUSE_ACTION_REMOVE: - iput(get_fuse_inode(inode)->backing_inode); - get_fuse_inode(inode)->backing_inode = NULL; - path_put_init(&get_fuse_dentry(entry)->backing_path); - break; - - case FUSE_ACTION_REPLACE: { - struct fuse_conn *fc; - struct file *backing_file; - - fc = get_fuse_mount(dir)->fc; - backing_file = feb->backing_file; - if (!backing_file || IS_ERR(backing_file)) - return ERR_PTR(-EIO); - - iput(get_fuse_inode(inode)->backing_inode); - get_fuse_inode(inode)->backing_inode = - backing_file->f_inode; - ihold(get_fuse_inode(inode)->backing_inode); - - path_put(&get_fuse_dentry(entry)->backing_path); - get_fuse_dentry(entry)->backing_path = backing_file->f_path; - path_get(&get_fuse_dentry(entry)->backing_path); - - fput(backing_file); - break; - } - - default: - return ERR_PTR(-EIO); - } + error = handle_inode_backing_fd(inode, entry, febo, feb); + if (error) + return ERR_PTR(error); get_fuse_inode(inode)->nodeid = feo->nodeid; diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 8a3e1fb9ab14..ff1087671067 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -238,25 +238,23 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags) ret = fuse_simple_request(fm, &args); dput(parent); - /* - * TODO This doesn't seem sufficient, though we don't plan to - * change the backing file ever, so not sure what is correct - * here yet, especially as we can't return an error to user - */ - if (bpf_arg.out.backing_action == FUSE_ACTION_REPLACE) { - struct file *file = bpf_arg.backing_file; +#ifdef CONFIG_FUSE_BPF + if (ret == sizeof(bpf_arg.out)) { + ret = -ENOENT; + if (!entry) + goto out; - if (file && !IS_ERR(file)) - fput(file); + ret = handle_inode_backing_fd(inode, entry, + &bpf_arg.out, &bpf_arg); + if (ret) + goto out; + + ret = handle_inode_bpf(inode, entry->d_parent->d_inode, + &bpf_arg.out, &bpf_arg); + if (ret) + goto out; } - - if (bpf_arg.out.bpf_action == FUSE_ACTION_REPLACE) { - struct file *file = bpf_arg.bpf_file; - - if (file && !IS_ERR(file)) - fput(file); - } - +#endif /* Zero nodeid is same as -ENOENT */ if (!ret && !outarg.nodeid) ret = -ENOENT; @@ -527,7 +525,6 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name #ifdef CONFIG_FUSE_BPF if (err == sizeof(bpf_arg.out)) { /* TODO Make sure this handles invalid handles */ - /* TODO Do we need the same code in revalidate */ struct file *backing_file; struct inode *backing_inode; @@ -536,37 +533,29 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name goto out_queue_forget; err = -EINVAL; - if (bpf_arg.out.backing_action != FUSE_ACTION_REPLACE) + backing_file = bpf_arg.backing_file; + if (!backing_file) goto out_queue_forget; - backing_file = bpf_arg.backing_file; - if (!backing_file || IS_ERR(backing_file)) + if (IS_ERR(backing_file)) { + err = PTR_ERR(backing_file); goto out_queue_forget; + } backing_inode = backing_file->f_inode; *inode = fuse_iget_backing(sb, outarg->nodeid, backing_inode); if (!*inode) goto bpf_arg_out; - if (bpf_arg.out.bpf_action == FUSE_ACTION_REPLACE) { - struct file *bpf_file = bpf_arg.bpf_file; - struct bpf_prog *bpf_prog = ERR_PTR(-EINVAL); - - if (bpf_file && !IS_ERR(bpf_file)) - bpf_prog = fuse_get_bpf_prog(bpf_file);; - - if (IS_ERR(bpf_prog)) { - iput(*inode); - *inode = NULL; - err = PTR_ERR(bpf_prog); - goto bpf_arg_out; - } - get_fuse_inode(*inode)->bpf = bpf_prog; - } - - get_fuse_dentry(entry)->backing_path = backing_file->f_path; - path_get(&get_fuse_dentry(entry)->backing_path); + err = handle_inode_backing_fd(*inode, entry, + &bpf_arg.out, &bpf_arg); + if (err) + goto out; + err = handle_inode_bpf(*inode, NULL, + &bpf_arg.out, &bpf_arg); + if (err) + goto out; bpf_arg_out: fput(backing_file); } else diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 22cf7ec88e04..aa4509e20235 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -1595,6 +1595,13 @@ struct fuse_lookup_io { struct fuse_entry_bpf feb; }; +int handle_inode_backing_fd(struct inode *inode, struct dentry *entry, + struct fuse_entry_bpf_out *febo, + struct fuse_entry_bpf *feb); +int handle_inode_bpf(struct inode *inode, struct inode *parent, + struct fuse_entry_bpf_out *febo, + struct fuse_entry_bpf *feb); + int fuse_lookup_initialize(struct fuse_bpf_args *fa, struct fuse_lookup_io *feo, struct inode *dir, struct dentry *entry, unsigned int flags); int fuse_lookup_backing(struct fuse_bpf_args *fa, struct inode *dir,