| Message ID | 20210125153057.3623715-8-balsini@android.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fuse: Add support for passthrough read/write | expand |
On Mon, Jan 25, 2021 at 11:31 PM Alessio Balsini <balsini@android.com> wrote: > > When using FUSE passthrough, read/write operations are directly > forwarded to the lower file system file through VFS, but there is no > guarantee that the process that is triggering the request has the right > permissions to access the lower file system. This would cause the > read/write access to fail. > > In passthrough file systems, where the FUSE daemon is responsible for > the enforcement of the lower file system access policies, often happens > that the process dealing with the FUSE file system doesn't have access > to the lower file system. > Being the FUSE daemon in charge of implementing the FUSE file > operations, that in the case of read/write operations usually simply > results in the copy of memory buffers from/to the lower file system > respectively, these operations are executed with the FUSE daemon > privileges. > > This patch adds a reference to the FUSE daemon credentials, referenced > at FUSE_DEV_IOC_PASSTHROUGH_OPEN ioctl() time so that they can be used > to temporarily raise the user credentials when accessing lower file > system files in passthrough. > The process accessing the FUSE file with passthrough enabled temporarily > receives the privileges of the FUSE daemon while performing read/write > operations. Similar behavior is implemented in overlayfs. > These privileges will be reverted as soon as the IO operation completes. > This feature does not provide any higher security privileges to those > processes accessing the FUSE file system with passthrough enabled. This > is because it is still the FUSE daemon responsible for enabling or not > the passthrough feature at file open time, and should enable the feature > only after appropriate access policy checks. > > Signed-off-by: Alessio Balsini <balsini@android.com> > --- > fs/fuse/fuse_i.h | 5 ++++- > fs/fuse/passthrough.c | 11 +++++++++++ > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h > index c4730d893324..815af1845b16 100644 > --- a/fs/fuse/fuse_i.h > +++ b/fs/fuse/fuse_i.h > @@ -182,10 +182,13 @@ struct fuse_release_args; > > /** > * Reference to lower filesystem file for read/write operations handled in > - * passthrough mode > + * passthrough mode. > + * This struct also tracks the credentials to be used for handling read/write > + * operations. > */ > struct fuse_passthrough { > struct file *filp; > + struct cred *cred; > }; > > /** FUSE specific file data */ > diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c > index c7fa1eeb7639..24866c5fe7e2 100644 > --- a/fs/fuse/passthrough.c > +++ b/fs/fuse/passthrough.c > @@ -52,6 +52,7 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb_fuse, > struct iov_iter *iter) > { > ssize_t ret; > + const struct cred *old_cred; > struct file *fuse_filp = iocb_fuse->ki_filp; > struct fuse_file *ff = fuse_filp->private_data; > struct file *passthrough_filp = ff->passthrough.filp; > @@ -59,6 +60,7 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb_fuse, > if (!iov_iter_count(iter)) > return 0; > > + old_cred = override_creds(ff->passthrough.cred); > if (is_sync_kiocb(iocb_fuse)) { > ret = vfs_iter_read(passthrough_filp, iter, &iocb_fuse->ki_pos, > iocb_to_rw_flags(iocb_fuse->ki_flags, > @@ -77,6 +79,7 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb_fuse, > if (ret != -EIOCBQUEUED) > fuse_aio_cleanup_handler(aio_req); > } > + revert_creds(old_cred); cred should be reverted when kmalloc() fails above. Cheers, Tao
On Fri, Feb 05, 2021 at 05:23:56PM +0800, Peng Tao wrote: > On Mon, Jan 25, 2021 at 11:31 PM Alessio Balsini <balsini@android.com> wrote: > > > > When using FUSE passthrough, read/write operations are directly > > forwarded to the lower file system file through VFS, but there is no > > guarantee that the process that is triggering the request has the right > > permissions to access the lower file system. This would cause the > > read/write access to fail. > > > > In passthrough file systems, where the FUSE daemon is responsible for > > the enforcement of the lower file system access policies, often happens > > that the process dealing with the FUSE file system doesn't have access > > to the lower file system. > > Being the FUSE daemon in charge of implementing the FUSE file > > operations, that in the case of read/write operations usually simply > > results in the copy of memory buffers from/to the lower file system > > respectively, these operations are executed with the FUSE daemon > > privileges. > > > > This patch adds a reference to the FUSE daemon credentials, referenced > > at FUSE_DEV_IOC_PASSTHROUGH_OPEN ioctl() time so that they can be used > > to temporarily raise the user credentials when accessing lower file > > system files in passthrough. > > The process accessing the FUSE file with passthrough enabled temporarily > > receives the privileges of the FUSE daemon while performing read/write > > operations. Similar behavior is implemented in overlayfs. > > These privileges will be reverted as soon as the IO operation completes. > > This feature does not provide any higher security privileges to those > > processes accessing the FUSE file system with passthrough enabled. This > > is because it is still the FUSE daemon responsible for enabling or not > > the passthrough feature at file open time, and should enable the feature > > only after appropriate access policy checks. > > > > Signed-off-by: Alessio Balsini <balsini@android.com> > > --- > > fs/fuse/fuse_i.h | 5 ++++- > > fs/fuse/passthrough.c | 11 +++++++++++ > > 2 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h > > index c4730d893324..815af1845b16 100644 > > --- a/fs/fuse/fuse_i.h > > +++ b/fs/fuse/fuse_i.h > > @@ -182,10 +182,13 @@ struct fuse_release_args; > > > > /** > > * Reference to lower filesystem file for read/write operations handled in > > - * passthrough mode > > + * passthrough mode. > > + * This struct also tracks the credentials to be used for handling read/write > > + * operations. > > */ > > struct fuse_passthrough { > > struct file *filp; > > + struct cred *cred; > > }; > > > > /** FUSE specific file data */ > > diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c > > index c7fa1eeb7639..24866c5fe7e2 100644 > > --- a/fs/fuse/passthrough.c > > +++ b/fs/fuse/passthrough.c > > @@ -52,6 +52,7 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb_fuse, > > struct iov_iter *iter) > > { > > ssize_t ret; > > + const struct cred *old_cred; > > struct file *fuse_filp = iocb_fuse->ki_filp; > > struct fuse_file *ff = fuse_filp->private_data; > > struct file *passthrough_filp = ff->passthrough.filp; > > @@ -59,6 +60,7 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb_fuse, > > if (!iov_iter_count(iter)) > > return 0; > > > > + old_cred = override_creds(ff->passthrough.cred); > > if (is_sync_kiocb(iocb_fuse)) { > > ret = vfs_iter_read(passthrough_filp, iter, &iocb_fuse->ki_pos, > > iocb_to_rw_flags(iocb_fuse->ki_flags, > > @@ -77,6 +79,7 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb_fuse, > > if (ret != -EIOCBQUEUED) > > fuse_aio_cleanup_handler(aio_req); > > } > > + revert_creds(old_cred); > cred should be reverted when kmalloc() fails above. > > Cheers, > Tao > -- > Into Sth. Rich & Strange Thanks Tao, definitely! Please find the fixup at the bottom of this email. I keep the WIP V13 here: https://github.com/balsini/linux/tree/fuse-passthrough-v13-v5.11-rc5 Thanks, Alessio ---8<--- From 63797a2cc6b3946bce59989adcb8f39f70f27643 Mon Sep 17 00:00:00 2001 From: Alessio Balsini <balsini@android.com> Date: Fri, 5 Feb 2021 10:58:49 +0000 Subject: [PATCH] fuse: Fix crediantials leak in passthrough read_iter If the system doesn't have enough memory when fuse_passthrough_read_iter is requested in asynchronous IO, an error is directly returned without restoring the caller's credentials. Fix by always ensuring credentials are restored. Fixes: 20210125153057.3623715-8-balsini@android.com ("fuse: Use daemon creds in passthrough mode") Signed-off-by: Alessio Balsini <balsini@android.com> --- fs/fuse/passthrough.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c index 284979f87747..1df94c1d8a00 100644 --- a/fs/fuse/passthrough.c +++ b/fs/fuse/passthrough.c @@ -69,8 +69,10 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb_fuse, struct fuse_aio_req *aio_req; aio_req = kmalloc(sizeof(struct fuse_aio_req), GFP_KERNEL); - if (!aio_req) - return -ENOMEM; + if (!aio_req) { + ret = -ENOMEM; + goto out; + } aio_req->iocb_fuse = iocb_fuse; kiocb_clone(&aio_req->iocb, iocb_fuse, passthrough_filp); @@ -79,6 +81,7 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb_fuse, if (ret != -EIOCBQUEUED) fuse_aio_cleanup_handler(aio_req); } +out: revert_creds(old_cred); return ret; -- 2.30.0.365.g02bc693789-goog
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index c4730d893324..815af1845b16 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -182,10 +182,13 @@ struct fuse_release_args; /** * Reference to lower filesystem file for read/write operations handled in - * passthrough mode + * passthrough mode. + * This struct also tracks the credentials to be used for handling read/write + * operations. */ struct fuse_passthrough { struct file *filp; + struct cred *cred; }; /** FUSE specific file data */ diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c index c7fa1eeb7639..24866c5fe7e2 100644 --- a/fs/fuse/passthrough.c +++ b/fs/fuse/passthrough.c @@ -52,6 +52,7 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb_fuse, struct iov_iter *iter) { ssize_t ret; + const struct cred *old_cred; struct file *fuse_filp = iocb_fuse->ki_filp; struct fuse_file *ff = fuse_filp->private_data; struct file *passthrough_filp = ff->passthrough.filp; @@ -59,6 +60,7 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb_fuse, if (!iov_iter_count(iter)) return 0; + old_cred = override_creds(ff->passthrough.cred); if (is_sync_kiocb(iocb_fuse)) { ret = vfs_iter_read(passthrough_filp, iter, &iocb_fuse->ki_pos, iocb_to_rw_flags(iocb_fuse->ki_flags, @@ -77,6 +79,7 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb_fuse, if (ret != -EIOCBQUEUED) fuse_aio_cleanup_handler(aio_req); } + revert_creds(old_cred); return ret; } @@ -85,6 +88,7 @@ ssize_t fuse_passthrough_write_iter(struct kiocb *iocb_fuse, struct iov_iter *iter) { ssize_t ret; + const struct cred *old_cred; struct file *fuse_filp = iocb_fuse->ki_filp; struct fuse_file *ff = fuse_filp->private_data; struct inode *fuse_inode = file_inode(fuse_filp); @@ -96,6 +100,7 @@ ssize_t fuse_passthrough_write_iter(struct kiocb *iocb_fuse, inode_lock(fuse_inode); + old_cred = override_creds(ff->passthrough.cred); if (is_sync_kiocb(iocb_fuse)) { file_start_write(passthrough_filp); ret = vfs_iter_write(passthrough_filp, iter, &iocb_fuse->ki_pos, @@ -124,6 +129,7 @@ ssize_t fuse_passthrough_write_iter(struct kiocb *iocb_fuse, fuse_aio_cleanup_handler(aio_req); } out: + revert_creds(old_cred); inode_unlock(fuse_inode); return ret; @@ -174,6 +180,7 @@ int fuse_passthrough_open(struct fuse_dev *fud, } passthrough->filp = passthrough_filp; + passthrough->cred = prepare_creds(); idr_preload(GFP_KERNEL); spin_lock(&fc->passthrough_req_lock); @@ -225,4 +232,8 @@ void fuse_passthrough_release(struct fuse_passthrough *passthrough) fput(passthrough->filp); passthrough->filp = NULL; } + if (passthrough->cred) { + put_cred(passthrough->cred); + passthrough->cred = NULL; + } }
When using FUSE passthrough, read/write operations are directly forwarded to the lower file system file through VFS, but there is no guarantee that the process that is triggering the request has the right permissions to access the lower file system. This would cause the read/write access to fail. In passthrough file systems, where the FUSE daemon is responsible for the enforcement of the lower file system access policies, often happens that the process dealing with the FUSE file system doesn't have access to the lower file system. Being the FUSE daemon in charge of implementing the FUSE file operations, that in the case of read/write operations usually simply results in the copy of memory buffers from/to the lower file system respectively, these operations are executed with the FUSE daemon privileges. This patch adds a reference to the FUSE daemon credentials, referenced at FUSE_DEV_IOC_PASSTHROUGH_OPEN ioctl() time so that they can be used to temporarily raise the user credentials when accessing lower file system files in passthrough. The process accessing the FUSE file with passthrough enabled temporarily receives the privileges of the FUSE daemon while performing read/write operations. Similar behavior is implemented in overlayfs. These privileges will be reverted as soon as the IO operation completes. This feature does not provide any higher security privileges to those processes accessing the FUSE file system with passthrough enabled. This is because it is still the FUSE daemon responsible for enabling or not the passthrough feature at file open time, and should enable the feature only after appropriate access policy checks. Signed-off-by: Alessio Balsini <balsini@android.com> --- fs/fuse/fuse_i.h | 5 ++++- fs/fuse/passthrough.c | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-)