| Message ID | 20230519125705.598234-11-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fuse: Add support for passthrough read/write | expand |
On Fri, 19 May 2023 at 14:57, Amir Goldstein <amir73il@gmail.com> wrote: > > WIP > > Add an ioctl to associate a FUSE server open fd with a request. > A later response to this request get use the FOPEN_PASSTHROUGH flag > to request passthrough to the associated backing file. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > > Miklos, > > After implementing refcounted backing files, I started to think how > to limit the server from mapping too many files. > > I wanted to limit the backing files mappings to the number of open fuse > files to simplify backing files accounting (i.e. open files are > effectively accounted to clients). > > It occured to me that creatig a 1-to-1 mapping between fuse files and > backing file ids is quite futile if there is no need to manage 1-to-many > backing file mappings. > > If only 1-to-1 mapping is desired, the proposed ioctl associates a > backing file with a pending request. The backing file will be kept > open for as long the request lives, or until its refcount is handed > over to the client, which can then use it to setup passthough to the > backing file without the intermediate idr array. I think I understand what the patch does, but what I don't understand is how this is going to solve the resource accounting problem. Can you elaborate? Thanks, Miklos
On Tue, Jun 6, 2023 at 1:23 PM Miklos Szeredi via fuse-devel <fuse-devel@lists.sourceforge.net> wrote: > > On Fri, 19 May 2023 at 14:57, Amir Goldstein <amir73il@gmail.com> wrote: > > > > WIP > > > > Add an ioctl to associate a FUSE server open fd with a request. > > A later response to this request get use the FOPEN_PASSTHROUGH flag > > to request passthrough to the associated backing file. > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > --- > > > > Miklos, > > > > After implementing refcounted backing files, I started to think how > > to limit the server from mapping too many files. > > > > I wanted to limit the backing files mappings to the number of open fuse > > files to simplify backing files accounting (i.e. open files are > > effectively accounted to clients). > > > > It occured to me that creatig a 1-to-1 mapping between fuse files and > > backing file ids is quite futile if there is no need to manage 1-to-many > > backing file mappings. > > > > If only 1-to-1 mapping is desired, the proposed ioctl associates a > > backing file with a pending request. The backing file will be kept > > open for as long the request lives, or until its refcount is handed > > over to the client, which can then use it to setup passthough to the > > backing file without the intermediate idr array. > > I think I understand what the patch does, but what I don't understand > is how this is going to solve the resource accounting problem. > > Can you elaborate? > It does not solve the resource accounting in the traditional way of limiting the number of open files to the resource limit of the server process. Instead, it has the similar effect of overlayfs pseudo files non accounting. A FUSE passthrough filesystem can contribute the same number of non accounted open fds as the number of FUSE fds accounted to different processes. A non privileged user can indirectly cause unaccounted open fds with a FUSE passthough fs in the exact same way that the same user can cause unaccounted open fds with an overlayfs mount if it can convince other users to open files on the FUSE/ovl that it has mounted. Am I making sense? One possible improvement to this API is to include the nodeid in FUSE_DEV_IOC_PASSTHROUGH_SETUP to make the mapping per-inode. If the mapping was already created during another open of the same inode, the setup would fail with a special error (EEXIST) so the caller would close the new backing fd, but the request already takes a reference to the backing fd of the inode, so FOPEN_PASSTHROUGH may proceed. On the last close, the backing fd refcount drops to zero and then is detached from the inode. A server that wants to manage the mapping lifetime can still use FUSE_DEV_IOC_PASSTHROUGH_OPEN/CLOSE to do so (with nodeid) argument. Thanks, Amir.
On Tue, 6 Jun 2023 at 13:00, Amir Goldstein <amir73il@gmail.com> wrote: > > On Tue, Jun 6, 2023 at 1:23 PM Miklos Szeredi via fuse-devel > <fuse-devel@lists.sourceforge.net> wrote: > > > > On Fri, 19 May 2023 at 14:57, Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > WIP > > > > > > Add an ioctl to associate a FUSE server open fd with a request. > > > A later response to this request get use the FOPEN_PASSTHROUGH flag > > > to request passthrough to the associated backing file. > > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > > --- > > > > > > Miklos, > > > > > > After implementing refcounted backing files, I started to think how > > > to limit the server from mapping too many files. > > > > > > I wanted to limit the backing files mappings to the number of open fuse > > > files to simplify backing files accounting (i.e. open files are > > > effectively accounted to clients). > > > > > > It occured to me that creatig a 1-to-1 mapping between fuse files and > > > backing file ids is quite futile if there is no need to manage 1-to-many > > > backing file mappings. > > > > > > If only 1-to-1 mapping is desired, the proposed ioctl associates a > > > backing file with a pending request. The backing file will be kept > > > open for as long the request lives, or until its refcount is handed > > > over to the client, which can then use it to setup passthough to the > > > backing file without the intermediate idr array. > > > > I think I understand what the patch does, but what I don't understand > > is how this is going to solve the resource accounting problem. > > > > Can you elaborate? > > > > It does not solve the resource accounting in the traditional way > of limiting the number of open files to the resource limit of the > server process. > > Instead, it has the similar effect of overlayfs pseudo files > non accounting. > > A FUSE passthrough filesystem can contribute the same number > of non accounted open fds as the number of FUSE fds accounted > to different processes. > > A non privileged user can indirectly cause unaccounted open fds > with a FUSE passthough fs in the exact same way that the same > user can cause unaccounted open fds with an overlayfs mount > if it can convince other users to open files on the FUSE/ovl that it > has mounted. > > Am I making sense? So this allows double the number of open files as normally would be allowed, same as overlayfs. Makes sense. Thanks, Miklos
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index cb00234e7843..01fb9c5411d2 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -61,6 +61,8 @@ static struct fuse_req *fuse_request_alloc(struct fuse_mount *fm, gfp_t flags) static void fuse_request_free(struct fuse_req *req) { + if (test_bit(FR_PASSTHROUGH, &req->flags)) + fuse_passthrough_put(req->fpt); kmem_cache_free(fuse_req_cachep, req); } @@ -2251,6 +2253,43 @@ static int fuse_device_clone(struct fuse_conn *fc, struct file *new) return 0; } +// Associate an passthrough fd to a fuse request +static int fuse_handle_ioc_passthrough_setup(struct fuse_dev *fud, + struct fuse_passthrough_setup_in __user *arg) +{ + struct fuse_pqueue *fpq = &fud->pq; + struct fuse_req *req; + struct fuse_passthrough_setup_in fpts; + struct fuse_passthrough *fpt; + int err; + + if (copy_from_user(&fpts, arg, sizeof(fpts))) + return -EFAULT; + + if (fpts.padding) + return -EINVAL; + + err = fuse_passthrough_open(fud->fc, fpts.fd, &fpt); + if (err) + return err; + + spin_lock(&fpq->lock); + req = NULL; + if (fpq->connected) + req = request_find(fpq, fpts.unique); + if (req) { + __set_bit(FR_PASSTHROUGH, &req->flags); + req->fpt = fpt; + } + spin_unlock(&fpq->lock); + if (!req) { + fuse_passthrough_put(fpt); + return -ENOENT; + } + + return 0; +} + static long fuse_dev_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -2291,7 +2330,7 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd, if (!f.file) return -EINVAL; - res = fuse_passthrough_open(fud->fc, fd); + res = fuse_passthrough_open(fud->fc, fd, NULL); fdput(f); break; case FUSE_DEV_IOC_PASSTHROUGH_CLOSE: @@ -2300,6 +2339,10 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd, res = fuse_passthrough_close(fud->fc, id); break; + case FUSE_DEV_IOC_PASSTHROUGH_SETUP: + res = fuse_handle_ioc_passthrough_setup(fud, + (struct fuse_passthrough_setup_in __user *)arg); + break; default: res = -ENOTTY; break; diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 238a43349298..085d7607ba6e 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -332,6 +332,7 @@ struct fuse_io_priv { * FR_FINISHED: request is finished * FR_PRIVATE: request is on private list * FR_ASYNC: request is asynchronous + * FR_PASSTHROUGH: request is associated with passthrough file */ enum fuse_req_flag { FR_ISREPLY, @@ -346,6 +347,7 @@ enum fuse_req_flag { FR_FINISHED, FR_PRIVATE, FR_ASYNC, + FR_PASSTHROUGH, }; /** @@ -385,10 +387,13 @@ struct fuse_req { /** Used to wake up the task waiting for completion of request*/ wait_queue_head_t waitq; -#if IS_ENABLED(CONFIG_VIRTIO_FS) - /** virtio-fs's physically contiguous buffer for in and out args */ - void *argbuf; -#endif + union { + /** virtio-fs's physically contiguous buffer for in/out args */ + void *argbuf; + + /** passthrough file associated with request */ + struct fuse_passthrough *fpt; + }; /** fuse_mount this request belongs to */ struct fuse_mount *fm; @@ -1347,7 +1352,8 @@ void fuse_file_release(struct inode *inode, struct fuse_file *ff, unsigned int open_flags, fl_owner_t id, bool isdir); /* passthrough.c */ -int fuse_passthrough_open(struct fuse_conn *fc, int backing_fd); +int fuse_passthrough_open(struct fuse_conn *fc, int backing_fd, + struct fuse_passthrough **pfpt); int fuse_passthrough_close(struct fuse_conn *fc, int passthrough_fh); int fuse_passthrough_setup(struct fuse_conn *fc, struct fuse_file *ff, struct fuse_open_out *openarg); diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c index 2b745b6b2364..85a1d72b1666 100644 --- a/fs/fuse/passthrough.c +++ b/fs/fuse/passthrough.c @@ -216,8 +216,12 @@ ssize_t fuse_passthrough_mmap(struct file *file, struct vm_area_struct *vma) /* * Returns passthrough_fh id that can be passed with FOPEN_PASSTHROUGH * open response and needs to be released with fuse_passthrough_close(). + * + * When @pfpt is non-NULL, an anonynous passthrough handle is returned + * via *pfpt on success and the return value is 0. */ -int fuse_passthrough_open(struct fuse_conn *fc, int backing_fd) +int fuse_passthrough_open(struct fuse_conn *fc, int backing_fd, + struct fuse_passthrough **pfpt) { struct file *passthrough_filp; struct inode *passthrough_inode; @@ -252,6 +256,12 @@ int fuse_passthrough_open(struct fuse_conn *fc, int backing_fd) passthrough->cred = prepare_creds(); refcount_set(&passthrough->count, 1); + if (pfpt) { + /* Return an anonynous passthrough handle */ + *pfpt = passthrough; + return 0; + } + idr_preload(GFP_KERNEL); spin_lock(&fc->lock); res = idr_alloc_cyclic(&fc->passthrough_files_map, passthrough, 1, 0, diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h index 3da1f59007cf..028a65fe3fa2 100644 --- a/include/uapi/linux/fuse.h +++ b/include/uapi/linux/fuse.h @@ -993,11 +993,19 @@ struct fuse_notify_retrieve_in { uint64_t dummy4; }; +struct fuse_passthrough_setup_in { + uint64_t unique; + uint32_t fd; + uint32_t padding; +}; + /* 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_PASSTHROUGH_OPEN _IOW(FUSE_DEV_IOC_MAGIC, 1, uint32_t) #define FUSE_DEV_IOC_PASSTHROUGH_CLOSE _IOW(FUSE_DEV_IOC_MAGIC, 2, uint32_t) +#define FUSE_DEV_IOC_PASSTHROUGH_SETUP _IOW(FUSE_DEV_IOC_MAGIC, 3, \ + struct fuse_passthrough_setup_in) struct fuse_lseek_in { uint64_t fh;
WIP Add an ioctl to associate a FUSE server open fd with a request. A later response to this request get use the FOPEN_PASSTHROUGH flag to request passthrough to the associated backing file. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- Miklos, After implementing refcounted backing files, I started to think how to limit the server from mapping too many files. I wanted to limit the backing files mappings to the number of open fuse files to simplify backing files accounting (i.e. open files are effectively accounted to clients). It occured to me that creatig a 1-to-1 mapping between fuse files and backing file ids is quite futile if there is no need to manage 1-to-many backing file mappings. If only 1-to-1 mapping is desired, the proposed ioctl associates a backing file with a pending request. The backing file will be kept open for as long the request lives, or until its refcount is handed over to the client, which can then use it to setup passthough to the backing file without the intermediate idr array. I have not implemented the full hand over yet, because I wanted to hear your feedback first. Thanks, Amir. fs/fuse/dev.c | 45 ++++++++++++++++++++++++++++++++++++++- fs/fuse/fuse_i.h | 16 +++++++++----- fs/fuse/passthrough.c | 12 ++++++++++- include/uapi/linux/fuse.h | 8 +++++++ 4 files changed, 74 insertions(+), 7 deletions(-)