OSDN Git Service

fuse: in fuse_flush only wait if someone wants the return code
authorEric W. Biederman <ebiederm@xmission.com>
Thu, 26 Jan 2023 16:10:58 +0000 (17:10 +0100)
committerMiklos Szeredi <mszeredi@redhat.com>
Thu, 26 Jan 2023 16:10:58 +0000 (17:10 +0100)
If a fuse filesystem is mounted inside a container, there is a problem
during pid namespace destruction. The scenario is:

1. task (a thread in the fuse server, with a fuse file open) starts
   exiting, does exit_signals(), goes into fuse_flush() -> wait
2. fuse daemon gets killed, tries to wake everyone up
3. task from 1 is stuck because complete_signal() doesn't wake it up, since
   it has PF_EXITING.

The result is that the thread will never be woken up, and pid namespace
destruction will block indefinitely.

To add insult to injury, nobody is waiting for these return codes, since
the pid namespace is being destroyed.

To fix this, let's not block on flush operations when the current task has
PF_EXITING.

This does change the semantics slightly: the wait here is for posix locks
to be unlocked, so the task will exit before things are unlocked. To quote
Miklos:

  "remote" posix locks are almost never used due to problems like this, so
  I think it's safe to do this.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
Link: https://lore.kernel.org/all/YrShFXRLtRt6T%2Fj+@risky/
Tested-by: Tycho Andersen <tycho@tycho.pizza>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
fs/fuse/file.c

index 875314e..ad5dc8f 100644 (file)
@@ -18,6 +18,7 @@
 #include <linux/falloc.h>
 #include <linux/uio.h>
 #include <linux/fs.h>
+#include <linux/file.h>
 
 static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
                          unsigned int open_flags, int opcode,
@@ -477,48 +478,36 @@ static void fuse_sync_writes(struct inode *inode)
        fuse_release_nowrite(inode);
 }
 
-static int fuse_flush(struct file *file, fl_owner_t id)
-{
-       struct inode *inode = file_inode(file);
-       struct fuse_mount *fm = get_fuse_mount(inode);
-       struct fuse_file *ff = file->private_data;
+struct fuse_flush_args {
+       struct fuse_args args;
        struct fuse_flush_in inarg;
-       FUSE_ARGS(args);
-       int err;
-
-       if (fuse_is_bad(inode))
-               return -EIO;
+       struct work_struct work;
+       struct file *file;
+};
 
-       if (ff->open_flags & FOPEN_NOFLUSH && !fm->fc->writeback_cache)
-               return 0;
+static int fuse_do_flush(struct fuse_flush_args *fa)
+{
+       int err;
+       struct inode *inode = file_inode(fa->file);
+       struct fuse_mount *fm = get_fuse_mount(inode);
 
        err = write_inode_now(inode, 1);
        if (err)
-               return err;
+               goto out;
 
        inode_lock(inode);
        fuse_sync_writes(inode);
        inode_unlock(inode);
 
-       err = filemap_check_errors(file->f_mapping);
+       err = filemap_check_errors(fa->file->f_mapping);
        if (err)
-               return err;
+               goto out;
 
        err = 0;
        if (fm->fc->no_flush)
                goto inval_attr_out;
 
-       memset(&inarg, 0, sizeof(inarg));
-       inarg.fh = ff->fh;
-       inarg.lock_owner = fuse_lock_owner_id(fm->fc, id);
-       args.opcode = FUSE_FLUSH;
-       args.nodeid = get_node_id(inode);
-       args.in_numargs = 1;
-       args.in_args[0].size = sizeof(inarg);
-       args.in_args[0].value = &inarg;
-       args.force = true;
-
-       err = fuse_simple_request(fm, &args);
+       err = fuse_simple_request(fm, &fa->args);
        if (err == -ENOSYS) {
                fm->fc->no_flush = 1;
                err = 0;
@@ -531,9 +520,57 @@ inval_attr_out:
         */
        if (!err && fm->fc->writeback_cache)
                fuse_invalidate_attr_mask(inode, STATX_BLOCKS);
+
+out:
+       fput(fa->file);
+       kfree(fa);
        return err;
 }
 
+static void fuse_flush_async(struct work_struct *work)
+{
+       struct fuse_flush_args *fa = container_of(work, typeof(*fa), work);
+
+       fuse_do_flush(fa);
+}
+
+static int fuse_flush(struct file *file, fl_owner_t id)
+{
+       struct fuse_flush_args *fa;
+       struct inode *inode = file_inode(file);
+       struct fuse_mount *fm = get_fuse_mount(inode);
+       struct fuse_file *ff = file->private_data;
+
+       if (fuse_is_bad(inode))
+               return -EIO;
+
+       if (ff->open_flags & FOPEN_NOFLUSH && !fm->fc->writeback_cache)
+               return 0;
+
+       fa = kzalloc(sizeof(*fa), GFP_KERNEL);
+       if (!fa)
+               return -ENOMEM;
+
+       fa->inarg.fh = ff->fh;
+       fa->inarg.lock_owner = fuse_lock_owner_id(fm->fc, id);
+       fa->args.opcode = FUSE_FLUSH;
+       fa->args.nodeid = get_node_id(inode);
+       fa->args.in_numargs = 1;
+       fa->args.in_args[0].size = sizeof(fa->inarg);
+       fa->args.in_args[0].value = &fa->inarg;
+       fa->args.force = true;
+       fa->file = get_file(file);
+
+       /* Don't wait if the task is exiting */
+       if (current->flags & PF_EXITING) {
+               INIT_WORK(&fa->work, fuse_flush_async);
+               schedule_work(&fa->work);
+               return 0;
+       }
+
+       return fuse_do_flush(fa);
+}
+
 int fuse_fsync_common(struct file *file, loff_t start, loff_t end,
                      int datasync, int opcode)
 {