shinys000114/linux
1
0
Fork 0
mirror of https://github.com/hardkernel/linux.git synced 2026-06-07 03:15:31 +09:00
Code Issues Packages Projects Releases Wiki Activity

ANDROID: fuse-bpf: Fix revalidate error path and backing handling

Browse Source 
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 <dimorinny@google.com>
Change-Id: Ifa62e56b42ca5580b25682eb5f16b5c91826cf49
This commit is contained in:
Dmitrii Merkurev
2022-07-08 23:38:18 +00:00
committed by Daniel Rosenberg
parent 329650e3b9
commit a06f77a0dd
4 changed files with 254 additions and 111 deletions
Download Patch File Download Diff File Expand all files Collapse all files

177
fs/fuse/backing.c
View File

@@ -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;

67
fs/fuse/dir.c
View File

@@ -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;
@@ -528,7 +526,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;
@@ -537,37 +534,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

7
fs/fuse/fuse_i.h
View File

@@ -1597,6 +1597,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,

114
tools/testing/selftests/filesystems/fuse/fuse_test.c
View File

@@ -1779,6 +1779,117 @@ out:
return result;
}
/*
* State:
* Original: dst/folder1/content.txt
* ^
* |
* |
* Backing: src/folder1/content.txt
*
* Step 1: open(folder1) - lookup folder1 with entry_timeout set to 0
* Step 2: open(folder1) - lookup folder1 again to trigger revalidate wich will
* set backing fd
*
* Check 1: cat(content.txt) - check not receiving call on the fuse daemon
* and content is the same
*/
static int bpf_test_revalidate_handle_backing_fd(const char *mount_dir)
{
const char *folder1 = "folder1";
const char *content_file = "content.txt";
const char *content = "hello world";
int result = TEST_FAILURE;
int fuse_dev = -1;
int src_fd = -1;
int content_fd = -1;
int pid = -1;
int status;
TESTSYSCALL(s_mkdir(s_path(s(ft_src), s(folder1)), 0777));
TEST(content_fd = s_creat(s_pathn(3, s(ft_src), s(folder1), s(content_file)), 0777),
content_fd != -1);
TESTEQUAL(write(content_fd, content, strlen(content)), strlen(content));
TESTSYSCALL(close(content_fd));
content_fd = -1;
TESTEQUAL(mount_fuse_no_init(mount_dir, -1, -1, &fuse_dev), 0);
FUSE_ACTION
int dst_folder1_fd = -1;
int dst_content_fd = -1;
int dst_content_read_size = -1;
char content_buffer[9] = {0};
// Step 1: Lookup folder1
TESTERR(dst_folder1_fd = s_open(s_path(s(mount_dir), s(folder1)),
O_RDONLY | O_CLOEXEC), dst_folder1_fd != -1);
TESTSYSCALL(close(dst_folder1_fd));
dst_folder1_fd = -1;
// Step 2: Lookup folder1 again
TESTERR(dst_folder1_fd = s_open(s_path(s(mount_dir), s(folder1)),
O_RDONLY | O_CLOEXEC), dst_folder1_fd != -1);
TESTSYSCALL(close(dst_folder1_fd));
dst_folder1_fd = -1;
// Check 1: Read content file (must be backed)
TESTERR(dst_content_fd =
s_open(s_pathn(3, s(mount_dir), s(folder1), s(content_file)),
O_RDONLY | O_CLOEXEC), dst_content_fd != -1);
TEST(dst_content_read_size =
read(dst_content_fd, content_buffer, strlen(content)),
dst_content_read_size == strlen(content) &&
strcmp(content, content_buffer) == 0);
TESTSYSCALL(close(dst_content_fd));
dst_content_fd = -1;
FUSE_DAEMON
struct fuse_attr attr = {};
int backing_fd = -1;
TESTFUSEINITFLAGS(FUSE_DO_READDIRPLUS | FUSE_READDIRPLUS_AUTO);
// Step 1: Lookup folder1 set entry_timeout to 0 to trigger
// revalidate later
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 = 0,
.attr_valid = UINT64_MAX,
.entry_valid_nsec = 0,
.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));
// Step 1: Lookup folder1 as a reaction to revalidate call
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));
FUSE_DONE
result = TEST_SUCCESS;
out:
close(fuse_dev);
close(content_fd);
close(src_fd);
umount(mount_dir);
return result;
}
static int parse_options(int argc, char *const *argv)
{
signed char c;
@@ -1884,7 +1995,8 @@ int main(int argc, char *argv[])
MAKE_TEST(bpf_test_statfs),
MAKE_TEST(bpf_test_lseek),
MAKE_TEST(bpf_test_readdirplus_not_overriding_backing),
MAKE_TEST(bpf_test_no_readdirplus_without_nodeid)
MAKE_TEST(bpf_test_no_readdirplus_without_nodeid),
MAKE_TEST(bpf_test_revalidate_handle_backing_fd)
};
#undef MAKE_TEST