Messages in this thread Patch in this message | | | Date | Fri, 3 Mar 2023 20:19:21 +0100 | Subject | Re: [RFC PATCH 7/9] fuse: add fuse device ioctl(FUSE_DEV_IOC_REINIT) | From | Bernd Schubert <> |
| |
On 2/20/23 20:37, Alexander Mikhalitsyn wrote: > This ioctl aborts fuse connection and then reinitializes it, > sends FUSE_INIT request to allow a new userspace daemon > to pick up the fuse connection. > > Cc: Miklos Szeredi <mszeredi@redhat.com> > Cc: Al Viro <viro@zeniv.linux.org.uk> > Cc: Amir Goldstein <amir73il@gmail.com> > Cc: Stéphane Graber <stgraber@ubuntu.com> > Cc: Seth Forshee <sforshee@kernel.org> > Cc: Christian Brauner <brauner@kernel.org> > Cc: Andrei Vagin <avagin@gmail.com> > Cc: Pavel Tikhomirov <ptikhomirov@virtuozzo.com> > Cc: linux-fsdevel@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: criu@openvz.org > Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com> > --- > fs/fuse/dev.c | 132 ++++++++++++++++++++++++++++++++++++++ > include/uapi/linux/fuse.h | 1 + > 2 files changed, 133 insertions(+) > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 737764c2295e..0f53ffd63957 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -2187,6 +2187,112 @@ void fuse_abort_conn(struct fuse_conn *fc) > } > EXPORT_SYMBOL_GPL(fuse_abort_conn); > > +static int fuse_reinit_conn(struct fuse_conn *fc) > +{ > + struct fuse_iqueue *fiq = &fc->iq; > + struct fuse_dev *fud; > + unsigned int i;
Assuming you have a malicious daemon that tries to cause bad behavior, only allow one ioctl at at time? I.e. add a value that reinit is in progress? And unset at the end of the function?
> + > + if (fc->conn_gen + 1 < fc->conn_gen) > + return -EOVERFLOW; > +
Add a comment, like
/* Unsets fc->connected and fiq->connected and ensures that no new requests can be queued */
?
> + fuse_abort_conn(fc); > + fuse_wait_aborted(fc); > + > + spin_lock(&fc->lock); > + if (fc->connected) { > + spin_unlock(&fc->lock); > + return -EINVAL; > + } > + > + if (fc->conn_gen + 1 < fc->conn_gen) { > + spin_unlock(&fc->lock); > + return -EOVERFLOW; > + } > + > + fc->conn_gen++; > + > + spin_lock(&fiq->lock); > + if (request_pending(fiq) || fiq->forget_list_tail != &fiq->forget_list_head) { > + spin_unlock(&fiq->lock); > + spin_unlock(&fc->lock); > + return -EINVAL; > + } > + > + if (&fuse_dev_fiq_ops != fiq->ops) { > + spin_unlock(&fiq->lock); > + spin_unlock(&fc->lock); > + return -EOPNOTSUPP; > + } > + > + fiq->connected = 1; > + spin_unlock(&fiq->lock); > + > + spin_lock(&fc->bg_lock); > + if (!list_empty(&fc->bg_queue)) { > + spin_unlock(&fc->bg_lock); > + spin_unlock(&fc->lock); > + return -EINVAL; > + } > + > + fc->blocked = 0; > + fc->max_background = FUSE_DEFAULT_MAX_BACKGROUND; > + spin_unlock(&fc->bg_lock); > + > + list_for_each_entry(fud, &fc->devices, entry) { > + struct fuse_pqueue *fpq = &fud->pq; > + > + spin_lock(&fpq->lock); > + if (!list_empty(&fpq->io)) { > + spin_unlock(&fpq->lock); > + spin_unlock(&fc->lock); > + return -EINVAL; > + } > + > + for (i = 0; i < FUSE_PQ_HASH_SIZE; i++) { > + if (!list_empty(&fpq->processing[i])) { > + spin_unlock(&fpq->lock); > + spin_unlock(&fc->lock); > + return -EINVAL; > + } > + } > + > + fpq->connected = 1; > + spin_unlock(&fpq->lock); > + } > + > + fuse_set_initialized(fc);
I'm not sure about this, why not the common way via FUSE_INIT reply?
> + > + /* Background queuing checks fc->connected under bg_lock */ > + spin_lock(&fc->bg_lock); > + fc->connected = 1; > + spin_unlock(&fc->bg_lock); > + > + fc->aborted = false; > + fc->abort_err = 0; > + > + /* nullify all the flags */ > + memset(&fc->flags, 0, sizeof(struct fuse_conn_flags)); > + > + spin_unlock(&fc->lock); > + > + down_read(&fc->killsb); > + if (!list_empty(&fc->mounts)) { > + struct fuse_mount *fm; > + > + fm = list_first_entry(&fc->mounts, struct fuse_mount, fc_entry); > + if (!fm->sb) { > + up_read(&fc->killsb); > + return -EINVAL; > + } > + > + fuse_send_init(fm); > + } > + up_read(&fc->killsb); > + > + return 0; > +} > + > void fuse_wait_aborted(struct fuse_conn *fc) > { > /* matches implicit memory barrier in fuse_drop_waiting() */ > @@ -2282,6 +2388,32 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd, > } > } > break; > + case FUSE_DEV_IOC_REINIT: > + struct fuse_conn *fc; > + > + if (!checkpoint_restore_ns_capable(file->f_cred->user_ns)) > + return -EPERM; > + > + res = -EINVAL; > + fud = fuse_get_dev(file); > + > + /* > + * Only fuse mounts with an already initialized fuse > + * connection are supported > + */ > + if (file->f_op == &fuse_dev_operations && fud) { > + mutex_lock(&fuse_mutex); > + fc = fud->fc; > + if (fc) > + fc = fuse_conn_get(fc); > + mutex_unlock(&fuse_mutex); > + > + if (fc) { > + res = fuse_reinit_conn(fc); > + fuse_conn_put(fc); > + } > + } > + break; > default: > res = -ENOTTY; > break; > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h > index 1b9d0dfae72d..3dac67b25eae 100644 > --- a/include/uapi/linux/fuse.h > +++ b/include/uapi/linux/fuse.h > @@ -989,6 +989,7 @@ struct fuse_notify_retrieve_in { > /* Device ioctls: */ > #define FUSE_DEV_IOC_MAGIC 229 > #define FUSE_DEV_IOC_CLONE _IOR(FUSE_DEV_IOC_MAGIC, 0, uint32_t) > +#define FUSE_DEV_IOC_REINIT _IO(FUSE_DEV_IOC_MAGIC, 0) > > struct fuse_lseek_in { > uint64_t fh;
| |