From bb1890afd7d1eb33e95f36d1d9936dbd574260b6 Mon Sep 17 00:00:00 2001 From: Peri Date: Thu, 11 May 2023 02:38:46 +0100 Subject: Fix issue #746. (#782) Added a secondary check in fuse_lib_unlink() after hide_node() to check again under a lock if the (now hidden) file is still open. If not then delete it. This should synchronise fuse_lib_unlink() with fuse_lib_release(), when nullpath_ok is set. --- lib/fuse.c | 14 ++++++ test/meson.build | 3 ++ test/release_unlink_race.c | 111 +++++++++++++++++++++++++++++++++++++++++++++ test/test_examples.py | 59 ++++++++++++++++++++++++ 4 files changed, 187 insertions(+) create mode 100644 test/release_unlink_race.c diff --git a/lib/fuse.c b/lib/fuse.c index a7feced..6d5df23 100644 --- a/lib/fuse.c +++ b/lib/fuse.c @@ -2967,6 +2967,20 @@ static void fuse_lib_unlink(fuse_req_t req, fuse_ino_t parent, fuse_prepare_interrupt(f, req, &d); if (!f->conf.hard_remove && is_open(f, parent, name)) { err = hide_node(f, path, parent, name); + if (!err) { + /* we have hidden the node so now check again under a lock in case it is not used any more */ + if (!is_open(f, parent, wnode->name)) { + char *unlinkpath; + + /* get the hidden file path, to unlink it */ + if (try_get_path(f, wnode->nodeid, NULL, &unlinkpath, NULL, false) == 0) { + err = fuse_fs_unlink(f->fs, unlinkpath); + if (!err) + remove_node(f, parent, wnode->name); + free(unlinkpath); + } + } + } } else { err = fuse_fs_unlink(f->fs, path); if (!err) diff --git a/test/meson.build b/test/meson.build index 1f5cb53..3d74b9a 100644 --- a/test/meson.build +++ b/test/meson.build @@ -13,6 +13,9 @@ td += executable('test_syscalls', 'test_syscalls.c', td += executable('readdir_inode', 'readdir_inode.c', include_directories: include_dirs, install: false) +td += executable('release_unlink_race', 'release_unlink_race.c', + dependencies: [ libfuse_dep ], + install: false) test_scripts = [ 'conftest.py', 'pytest.ini', 'test_examples.py', 'util.py', 'test_ctests.py', 'test_custom_io.py' ] diff --git a/test/release_unlink_race.c b/test/release_unlink_race.c new file mode 100644 index 0000000..2edb200 --- /dev/null +++ b/test/release_unlink_race.c @@ -0,0 +1,111 @@ +/* + This program can be distributed under the terms of the GNU GPLv2. + See the file COPYING. +*/ + +#define FUSE_USE_VERSION 31 + +#define _GNU_SOURCE + +#include + +#include +#include +#include +#include + +static void *xmp_init(struct fuse_conn_info *conn, + struct fuse_config *cfg) +{ + (void) conn; + + cfg->use_ino = 1; + cfg->nullpath_ok = 1; + cfg->entry_timeout = 0; + cfg->attr_timeout = 0; + cfg->negative_timeout = 0; + + return NULL; +} + +static int xmp_getattr(const char *path, struct stat *stbuf, + struct fuse_file_info *fi) +{ + int res; + + (void) path; + + if(fi) + res = fstat(fi->fh, stbuf); + else + res = lstat(path, stbuf); + if (res == -1) + return -errno; + + return 0; +} + +static int xmp_unlink(const char *path) +{ + int res; + + res = unlink(path); + if (res == -1) + return -errno; + + return 0; +} + +static int xmp_rename(const char *from, const char *to, unsigned int flags) +{ + int res; + + if (flags) + return -EINVAL; + + if(!getenv("RELEASEUNLINKRACE_DELAY_DISABLE")) usleep(100000); + + res = rename(from, to); + if (res == -1) + return -errno; + + return 0; +} + +static int xmp_create(const char *path, mode_t mode, struct fuse_file_info *fi) +{ + int fd; + + fd = open(path, fi->flags, mode); + if (fd == -1) + return -errno; + + fi->fh = fd; + return 0; +} + +static int xmp_release(const char *path, struct fuse_file_info *fi) +{ + (void) path; + + if(!getenv("RELEASEUNLINKRACE_DELAY_DISABLE")) usleep(100000); + + close(fi->fh); + + return 0; +} + +static const struct fuse_operations xmp_oper = { + .init = xmp_init, + .getattr = xmp_getattr, + .unlink = xmp_unlink, + .rename = xmp_rename, + .create = xmp_create, + .release = xmp_release, +}; + +int main(int argc, char *argv[]) +{ + umask(0); + return fuse_main(argc, argv, &xmp_oper, NULL); +} diff --git a/test/test_examples.py b/test/test_examples.py index f0aa63d..958e633 100755 --- a/test/test_examples.py +++ b/test/test_examples.py @@ -442,6 +442,65 @@ def test_cuse(output_checker): finally: mount_process.terminate() +def test_release_unlink_race(tmpdir, output_checker): + """test case for Issue #746 + + If RELEASE and UNLINK opcodes are sent back to back, and fuse_fs_release() + and fuse_fs_rename() are slow to execute, UNLINK will run while RELEASE is + still executing. UNLINK will try to rename the file and, while the rename + is happening, the RELEASE will finish executing. As a result, RELEASE will + not detect in time that UNLINK has happened, and UNLINK will not detect in + time that RELEASE has happened. + + + NOTE: This is triggered only when nullpath_ok is set. + + If it is NOT SET then get_path_nullok() called by fuse_lib_release() will + call get_path_common() and lock the path, and then the fuse_lib_unlink() + will wait for the path to be unlocked before executing and thus synchronise + with fuse_lib_release(). + + If it is SET then get_path_nullok() will just set the path to null and + return without locking anything and thus allowing fuse_lib_unlink() to + eventually execute unimpeded while fuse_lib_release() is still running. + """ + + fuse_mountpoint = str(tmpdir) + + fuse_binary_command = base_cmdline + \ + [ pjoin(basename, 'test', 'release_unlink_race'), + "-f", fuse_mountpoint] + + fuse_process = subprocess.Popen(fuse_binary_command, + stdout=output_checker.fd, + stderr=output_checker.fd) + + try: + wait_for_mount(fuse_process, fuse_mountpoint) + + temp_dir = tempfile.TemporaryDirectory(dir="/tmp/") + temp_dir_path = temp_dir.name + + fuse_temp_file, fuse_temp_file_path = tempfile.mkstemp(dir=(fuse_mountpoint + temp_dir_path)) + + os.close(fuse_temp_file) + os.unlink(fuse_temp_file_path) + + # needed for slow CI/CD pipelines for unlink OP to complete processing + safe_sleep(3) + + assert os.listdir(temp_dir_path) == [] + + except: + temp_dir.cleanup() + cleanup(fuse_process, fuse_mountpoint) + raise + + else: + temp_dir.cleanup() + umount(fuse_process, fuse_mountpoint) + + @contextmanager def os_open(name, flags): fd = os.open(name, flags) -- cgit v1.2.1