Add a test that mounts a libfuse3 server which can be told to refuse a request for an inode the client still holds a reference to, and check that the caller recovers instead of being told the file is gone.
The server answers ENOENT on demand for each request the fix converts, FUSE_OPEN, FUSE_GETATTR, FUSE_SETATTR, FUSE_READLINK and FUSE_STATFS, while continuing to serve every other request for the same inode, which is how a server that releases an inode too early behaves. open(), stat(), chmod(), readlink() and statfs() are then expected to succeed, having resolved the name again under LOOKUP_REVAL, and the request counters confirm the retry happened rather than the answer being served from cache. Each of those requests carries a nodeid, so an ENOENT answering one of them describes a handle rather than a name. FUSE_LOOKUP is the exception and is covered the other way round: a name the server does not have must still report ENOENT, since there the refusal is the answer. Signed-off-by: Aaron Paterson <[email protected]> --- .../selftests/filesystems/fuse/.gitignore | 1 + .../selftests/filesystems/fuse/Makefile | 4 + .../filesystems/fuse/fuse_estale_test.c | 450 ++++++++++++++++++ 3 files changed, 455 insertions(+) create mode 100644 tools/testing/selftests/filesystems/fuse/fuse_estale_test.c diff --git a/tools/testing/selftests/filesystems/fuse/.gitignore b/tools/testing/selftests/filesystems/fuse/.gitignore index 25c779065806..9cb3048128d5 100644 --- a/tools/testing/selftests/filesystems/fuse/.gitignore +++ b/tools/testing/selftests/filesystems/fuse/.gitignore @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only fuse_acl_cache_test fuse_mnt +fuse_estale_test fusectl_test write_extend_eof_test diff --git a/tools/testing/selftests/filesystems/fuse/Makefile b/tools/testing/selftests/filesystems/fuse/Makefile index 1ea87008ef9e..f564cb37b3a5 100644 --- a/tools/testing/selftests/filesystems/fuse/Makefile +++ b/tools/testing/selftests/filesystems/fuse/Makefile @@ -11,6 +11,7 @@ FUSE3_CFLAGS := $(shell pkg-config fuse3 --cflags 2>/dev/null) FUSE3_LDLIBS := $(shell pkg-config fuse3 --libs 2>/dev/null) ifneq ($(FUSE3_CFLAGS),) TEST_GEN_PROGS += fuse_acl_cache_test +TEST_GEN_PROGS += fuse_estale_test endif include ../../lib.mk @@ -32,3 +33,6 @@ $(OUTPUT)/fuse_mnt: LDLIBS += $(VAR_LDLIBS) $(OUTPUT)/fuse_acl_cache_test: CFLAGS += $(FUSE3_CFLAGS) $(OUTPUT)/fuse_acl_cache_test: LDLIBS += $(FUSE3_LDLIBS) + +$(OUTPUT)/fuse_estale_test: CFLAGS += $(FUSE3_CFLAGS) +$(OUTPUT)/fuse_estale_test: LDLIBS += $(FUSE3_LDLIBS) diff --git a/tools/testing/selftests/filesystems/fuse/fuse_estale_test.c b/tools/testing/selftests/filesystems/fuse/fuse_estale_test.c new file mode 100644 index 000000000000..82b842d3a5f7 --- /dev/null +++ b/tools/testing/selftests/filesystems/fuse/fuse_estale_test.c @@ -0,0 +1,450 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Test: a request refused for an inode the client still holds a reference to + * + * FUSE_OPEN, FUSE_GETATTR, FUSE_SETATTR, FUSE_READLINK and FUSE_STATFS carry a + * nodeid rather than a path. The client only sends them for an inode it has + * already looked up and holds a reference to, and a server owes the client that + * inode until it is sent FUSE_FORGET. A server that lets the inode go early, + * as one backing a shared directory does when the name is renamed over, answers + * with ENOENT. + * + * On an unfixed kernel that ENOENT is passed out unchanged. The path walk has + * no reason to doubt it and the caller is told a file is missing when it never + * stopped existing. Callers that read a missing file as an empty one act on + * the emptiness. + * + * Fixed (fs/fuse/file.c and fs/fuse/dir.c): ENOENT becomes ESTALE, which + * describes the handle rather than the name. filename_lookup() and + * do_filp_open() already retry with LOOKUP_REVAL on ESTALE, so the name is + * resolved again and the inode it refers to now is used. A name that has + * genuinely gone away fails the retried lookup, so ENOENT still reaches a + * caller that deserves it. + * + * Only requests reachable through a path walk are covered, because the retry + * is what makes ESTALE useful and the walk is what performs it. An operation + * on a descriptor already open has no equivalent recovery. + * + * Test outline: + * 1. Mount a minimal FUSE fs holding one file. + * 2. The server refuses the first request of the kind under test and allows + * every one after it, standing in for a server that released the inode and + * has since resolved the name again. + * 3. openat() the file. + * Buggy: ENOENT reaches the caller, one open was asked for. FAIL. + * Fixed: the walk retries, the second open is allowed, the descriptor is + * returned, two opens were asked for. PASS. + * 4. stat(), chmod(), readlink() and statfs() by name, which are the same + * recovery through FUSE_GETATTR, FUSE_SETATTR, FUSE_READLINK and + * FUSE_STATFS. Each is reached through a path walk, which is what makes + * the retry available. + * 5. Open a name the server does not have at all. + * Both: ENOENT, because the lookup fails rather than the open, and a + * file that is absent must still look absent. + */ + +#define _GNU_SOURCE +#define FUSE_USE_VERSION 34 + +#include <errno.h> +#include <fcntl.h> +#include <fuse_lowlevel.h> +#include <linux/limits.h> +#include <pthread.h> +#include <stdio.h> +#include <stdlib.h> +#include <stdbool.h> +#include <string.h> +#include <sys/stat.h> +#include <sys/statvfs.h> +#include <sys/vfs.h> +#include <unistd.h> + +#include "../../kselftest_harness.h" + +#define FILE_NAME "held" +#define LINK_NAME "held-link" +#define ABSENT_NAME "no-such-file" +#define FILE_INO 2 +#define LINK_INO 3 +#define CONTENTS "present\n" +#define LINK_TARGET FILE_NAME + +/* + * Which request the server refuses, and how many times. Shared with the + * daemon thread; one test runs at a time, so plain ints. + * + * Every one of these names an inode by nodeid rather than by name, so a + * refusal of any of them is describing a handle rather than a missing file. + * FUSE_LOOKUP is deliberately absent: it carries a name, so its ENOENT is an + * answer rather than a fault, and absent_name_still_reports_absent covers it. + */ +enum refuse_what { + REFUSE_NOTHING, + REFUSE_OPEN, + REFUSE_GETATTR, + REFUSE_SETATTR, + REFUSE_READLINK, + REFUSE_STATFS, +}; + +static struct { + enum refuse_what what; + int refusals_left; + int opens_seen; + int getattrs_seen; + int setattrs_seen; + int readlinks_seen; + int statfss_seen; +} g_ds; + +/* True once, for the request under test, and then never again. */ +static bool refuse_now(enum refuse_what what) +{ + if (g_ds.what != what || g_ds.refusals_left <= 0) + return false; + g_ds.refusals_left--; + return true; +} + +static void fill_attr(fuse_ino_t ino, struct stat *st) +{ + memset(st, 0, sizeof(*st)); + st->st_ino = ino; + /* + * Owned by whoever runs the test, so that chmod() is a request the + * kernel will carry through to the server rather than refuse itself. + */ + st->st_uid = getuid(); + st->st_gid = getgid(); + if (ino == FUSE_ROOT_ID) { + st->st_mode = S_IFDIR | 0755; + st->st_nlink = 2; + } else if (ino == LINK_INO) { + st->st_mode = S_IFLNK | 0777; + st->st_nlink = 1; + st->st_size = sizeof(LINK_TARGET) - 1; + } else { + st->st_mode = S_IFREG | 0644; + st->st_nlink = 1; + st->st_size = sizeof(CONTENTS) - 1; + } +} + +static void t_lookup(fuse_req_t req, fuse_ino_t parent, const char *name) +{ + struct fuse_entry_param e; + fuse_ino_t ino; + + if (parent != FUSE_ROOT_ID) + ino = 0; + else if (!strcmp(name, FILE_NAME)) + ino = FILE_INO; + else if (!strcmp(name, LINK_NAME)) + ino = LINK_INO; + else + ino = 0; + + if (!ino) { + fuse_reply_err(req, ENOENT); + return; + } + + memset(&e, 0, sizeof(e)); + e.ino = ino; + e.attr_timeout = 0; + e.entry_timeout = 0; + fill_attr(ino, &e.attr); + fuse_reply_entry(req, &e); +} + +static void t_getattr(fuse_req_t req, fuse_ino_t ino, + struct fuse_file_info *fi) +{ + struct stat st; + + (void)fi; + /* The root is left alone; refusing it would break the mount itself. */ + if (ino == FILE_INO) { + g_ds.getattrs_seen++; + if (refuse_now(REFUSE_GETATTR)) { + fuse_reply_err(req, ENOENT); + return; + } + } + fill_attr(ino, &st); + fuse_reply_attr(req, &st, 0); +} + +static void t_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi) +{ + if (ino != FILE_INO) { + fuse_reply_err(req, ENOENT); + return; + } + + g_ds.opens_seen++; + if (refuse_now(REFUSE_OPEN)) { + /* + * The inode is gone as far as this server is concerned, even + * though the client is holding a reference to it and asked by + * nodeid rather than by name. + */ + fuse_reply_err(req, ENOENT); + return; + } + fuse_reply_open(req, fi); +} + +static void t_read(fuse_req_t req, fuse_ino_t ino, size_t size, off_t off, + struct fuse_file_info *fi) +{ + size_t len = sizeof(CONTENTS) - 1; + + (void)fi; + if (ino != FILE_INO) { + fuse_reply_err(req, ENOENT); + return; + } + if ((size_t)off >= len) { + fuse_reply_buf(req, NULL, 0); + return; + } + if (off + size > len) + size = len - off; + fuse_reply_buf(req, CONTENTS + off, size); +} + +static void t_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, + int to_set, struct fuse_file_info *fi) +{ + struct stat st; + + (void)attr; + (void)to_set; + (void)fi; + if (ino == FILE_INO) { + g_ds.setattrs_seen++; + if (refuse_now(REFUSE_SETATTR)) { + fuse_reply_err(req, ENOENT); + return; + } + } + fill_attr(ino, &st); + fuse_reply_attr(req, &st, 0); +} + +static void t_readlink(fuse_req_t req, fuse_ino_t ino) +{ + if (ino != LINK_INO) { + fuse_reply_err(req, EINVAL); + return; + } + + g_ds.readlinks_seen++; + if (refuse_now(REFUSE_READLINK)) { + fuse_reply_err(req, ENOENT); + return; + } + fuse_reply_readlink(req, LINK_TARGET); +} + +static void t_statfs(fuse_req_t req, fuse_ino_t ino) +{ + struct statvfs sfs; + + (void)ino; + g_ds.statfss_seen++; + if (refuse_now(REFUSE_STATFS)) { + fuse_reply_err(req, ENOENT); + return; + } + + memset(&sfs, 0, sizeof(sfs)); + sfs.f_bsize = 512; + sfs.f_frsize = 512; + sfs.f_namemax = NAME_MAX; + fuse_reply_statfs(req, &sfs); +} + +static const struct fuse_lowlevel_ops fs_ops = { + .lookup = t_lookup, + .getattr = t_getattr, + .setattr = t_setattr, + .readlink = t_readlink, + .statfs = t_statfs, + .open = t_open, + .read = t_read, +}; + +static void *run_daemon(void *arg) +{ + fuse_session_loop((struct fuse_session *)arg); + return NULL; +} + +/* ---- kselftest harness --------------------------------------------------- */ + +FIXTURE(open_estale) { + struct fuse_session *se; + char mountpoint[PATH_MAX]; + char file_path[PATH_MAX]; + char link_path[PATH_MAX]; + char absent_path[PATH_MAX]; + pthread_t thread; +}; + +FIXTURE_SETUP(open_estale) +{ + char *fuse_argv[] = { "fuse_estale_test", NULL }; + struct fuse_args args = FUSE_ARGS_INIT(1, fuse_argv); + + memset(&g_ds, 0, sizeof(g_ds)); + g_ds.what = REFUSE_NOTHING; + g_ds.refusals_left = 1; + + strcpy(self->mountpoint, "/tmp/open_estale_test_XXXXXX"); + if (!mkdtemp(self->mountpoint)) + SKIP(return, "mkdtemp: %s", strerror(errno)); + + snprintf(self->file_path, sizeof(self->file_path), + "%s/" FILE_NAME, self->mountpoint); + snprintf(self->link_path, sizeof(self->link_path), + "%s/" LINK_NAME, self->mountpoint); + snprintf(self->absent_path, sizeof(self->absent_path), + "%s/" ABSENT_NAME, self->mountpoint); + + self->se = fuse_session_new(&args, &fs_ops, sizeof(fs_ops), NULL); + if (!self->se) { + rmdir(self->mountpoint); + SKIP(return, "fuse_session_new failed"); + } + + if (fuse_session_mount(self->se, self->mountpoint)) { + fuse_session_destroy(self->se); + rmdir(self->mountpoint); + SKIP(return, "fuse_session_mount failed (no fusermount3 or no privileges)"); + } + + if (pthread_create(&self->thread, NULL, run_daemon, self->se)) { + fuse_session_unmount(self->se); + fuse_session_destroy(self->se); + rmdir(self->mountpoint); + SKIP(return, "pthread_create: %s", strerror(errno)); + } + + fuse_opt_free_args(&args); +} + +FIXTURE_TEARDOWN(open_estale) +{ + fuse_session_exit(self->se); + fuse_session_unmount(self->se); + pthread_join(self->thread, NULL); + fuse_session_destroy(self->se); + rmdir(self->mountpoint); +} + +TEST_F(open_estale, refused_open_is_retried) +{ + int fd; + + g_ds.what = REFUSE_OPEN; + + fd = open(self->file_path, O_RDONLY); + + /* + * The refusal describes a handle the server should have honoured, so + * the walk is entitled to resolve the name again and open what it + * refers to now. Reporting the file missing instead ends the walk. + */ + ASSERT_GE(fd, 0) { + TH_LOG("open failed with %s after %d open request(s)", + strerror(errno), g_ds.opens_seen); + } + EXPECT_EQ(2, g_ds.opens_seen); + close(fd); +} + +TEST_F(open_estale, refused_getattr_on_path_is_retried) +{ + struct stat st; + + g_ds.what = REFUSE_GETATTR; + + /* + * Reached by name, so the walk can resolve it again and ask a second + * time, the same recovery the open gets. + */ + ASSERT_EQ(0, stat(self->file_path, &st)) { + TH_LOG("stat failed with %s after %d getattr request(s)", + strerror(errno), g_ds.getattrs_seen); + } + EXPECT_GT(g_ds.getattrs_seen, 1); +} + +TEST_F(open_estale, refused_setattr_on_path_is_retried) +{ + g_ds.what = REFUSE_SETATTR; + + /* + * chmod() reaches the inode by name, so the same retry applies: the + * refusal describes a handle and the walk may resolve the name again. + */ + ASSERT_EQ(0, chmod(self->file_path, 0600)) { + TH_LOG("chmod failed with %s after %d setattr request(s)", + strerror(errno), g_ds.setattrs_seen); + } + EXPECT_GT(g_ds.setattrs_seen, 1); +} + +TEST_F(open_estale, refused_readlink_on_path_is_retried) +{ + char buf[PATH_MAX]; + ssize_t n; + + g_ds.what = REFUSE_READLINK; + + n = readlink(self->link_path, buf, sizeof(buf) - 1); + ASSERT_GE(n, 0) { + TH_LOG("readlink failed with %s after %d readlink request(s)", + strerror(errno), g_ds.readlinks_seen); + } + buf[n] = '\0'; + EXPECT_STREQ(LINK_TARGET, buf); + EXPECT_GT(g_ds.readlinks_seen, 1); +} + +TEST_F(open_estale, refused_statfs_on_path_is_retried) +{ + struct statfs sfs; + + g_ds.what = REFUSE_STATFS; + + /* + * statfs() describes the mount rather than the file, but it is still + * reached through a path walk, so a refusal that names a handle is + * retried the same way. + */ + ASSERT_EQ(0, statfs(self->file_path, &sfs)) { + TH_LOG("statfs failed with %s after %d statfs request(s)", + strerror(errno), g_ds.statfss_seen); + } + EXPECT_GT(g_ds.statfss_seen, 1); +} + +TEST_F(open_estale, absent_name_still_reports_absent) +{ + int fd; + + /* + * Here it is the lookup that fails rather than the open, so nothing is + * being described as stale and the caller must still be told the name + * is not there. + */ + fd = open(self->absent_path, O_RDONLY); + ASSERT_LT(fd, 0); + EXPECT_EQ(ENOENT, errno); +} + +TEST_HARNESS_MAIN -- 2.55.0.553.g4ad8c266be
