From 3b6b99ef15ea37635604992ede9ebcccef38a239 Mon Sep 17 00:00:00 2001 From: Oleksandr Tymoshenko Date: Wed, 30 Oct 2024 00:28:55 +0000 Subject: [PATCH 01/13] ovl: properly handle large files in ovl_security_fileattr dentry_open in ovl_security_fileattr fails for any file larger than 2GB if open method of the underlying filesystem calls generic_file_open (e.g. fusefs). The issue can be reproduce using the following script: (passthrough_ll is an example app from libfuse). $ D=/opt/test/mnt $ mkdir -p ${D}/{source,base,top/uppr,top/work,ovlfs} $ dd if=/dev/zero of=${D}/source/zero.bin bs=1G count=2 $ passthrough_ll -o source=${D}/source ${D}/base $ mount -t overlay overlay \ -olowerdir=${D}/base,upperdir=${D}/top/uppr,workdir=${D}/top/work \ ${D}/ovlfs $ chmod 0777 ${D}/mnt/ovlfs/zero.bin Running this script results in "Value too large for defined data type" error message from chmod. Signed-off-by: Oleksandr Tymoshenko Fixes: 72db82115d2b ("ovl: copy up sync/noatime fileattr flags") Cc: stable@vger.kernel.org # v5.15+ Signed-off-by: Amir Goldstein --- fs/overlayfs/inode.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 35fd3e3e17780..baa54c718bd72 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -616,8 +616,13 @@ static int ovl_security_fileattr(const struct path *realpath, struct fileattr *f struct file *file; unsigned int cmd; int err; + unsigned int flags; + + flags = O_RDONLY; + if (force_o_largefile()) + flags |= O_LARGEFILE; - file = dentry_open(realpath, O_RDONLY, current_cred()); + file = dentry_open(realpath, flags, current_cred()); if (IS_ERR(file)) return PTR_ERR(file); From 48b50624aec454ce0fa8f78ef96e2f43bc0be495 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Mon, 21 Oct 2024 12:33:38 +0200 Subject: [PATCH 02/13] backing-file: clean up the API - Pass iocb to ctx->end_write() instead of file + pos - Get rid of ctx->user_file, which is redundant most of the time - Instead pass iocb to backing_file_splice_read and backing_file_splice_write Signed-off-by: Miklos Szeredi Signed-off-by: Amir Goldstein --- fs/backing-file.c | 33 ++++++++++++++++----------------- fs/fuse/passthrough.c | 32 ++++++++++++++++++-------------- fs/overlayfs/file.c | 22 +++++++++++++--------- include/linux/backing-file.h | 11 +++++------ 4 files changed, 52 insertions(+), 46 deletions(-) diff --git a/fs/backing-file.c b/fs/backing-file.c index 09a9be945d45e..a38737592ec77 100644 --- a/fs/backing-file.c +++ b/fs/backing-file.c @@ -80,7 +80,7 @@ struct backing_aio { refcount_t ref; struct kiocb *orig_iocb; /* used for aio completion */ - void (*end_write)(struct file *, loff_t, ssize_t); + void (*end_write)(struct kiocb *iocb, ssize_t); struct work_struct work; long res; }; @@ -108,10 +108,10 @@ static void backing_aio_cleanup(struct backing_aio *aio, long res) struct kiocb *iocb = &aio->iocb; struct kiocb *orig_iocb = aio->orig_iocb; + orig_iocb->ki_pos = iocb->ki_pos; if (aio->end_write) - aio->end_write(orig_iocb->ki_filp, iocb->ki_pos, res); + aio->end_write(orig_iocb, res); - orig_iocb->ki_pos = iocb->ki_pos; backing_aio_put(aio); } @@ -200,7 +200,7 @@ ssize_t backing_file_read_iter(struct file *file, struct iov_iter *iter, revert_creds(old_cred); if (ctx->accessed) - ctx->accessed(ctx->user_file); + ctx->accessed(iocb->ki_filp); return ret; } @@ -219,7 +219,7 @@ ssize_t backing_file_write_iter(struct file *file, struct iov_iter *iter, if (!iov_iter_count(iter)) return 0; - ret = file_remove_privs(ctx->user_file); + ret = file_remove_privs(iocb->ki_filp); if (ret) return ret; @@ -239,7 +239,7 @@ ssize_t backing_file_write_iter(struct file *file, struct iov_iter *iter, ret = vfs_iter_write(file, iter, &iocb->ki_pos, rwf); if (ctx->end_write) - ctx->end_write(ctx->user_file, iocb->ki_pos, ret); + ctx->end_write(iocb, ret); } else { struct backing_aio *aio; @@ -270,7 +270,7 @@ ssize_t backing_file_write_iter(struct file *file, struct iov_iter *iter, } EXPORT_SYMBOL_GPL(backing_file_write_iter); -ssize_t backing_file_splice_read(struct file *in, loff_t *ppos, +ssize_t backing_file_splice_read(struct file *in, struct kiocb *iocb, struct pipe_inode_info *pipe, size_t len, unsigned int flags, struct backing_file_ctx *ctx) @@ -282,19 +282,19 @@ ssize_t backing_file_splice_read(struct file *in, loff_t *ppos, return -EIO; old_cred = override_creds(ctx->cred); - ret = vfs_splice_read(in, ppos, pipe, len, flags); + ret = vfs_splice_read(in, &iocb->ki_pos, pipe, len, flags); revert_creds(old_cred); if (ctx->accessed) - ctx->accessed(ctx->user_file); + ctx->accessed(iocb->ki_filp); return ret; } EXPORT_SYMBOL_GPL(backing_file_splice_read); ssize_t backing_file_splice_write(struct pipe_inode_info *pipe, - struct file *out, loff_t *ppos, size_t len, - unsigned int flags, + struct file *out, struct kiocb *iocb, + size_t len, unsigned int flags, struct backing_file_ctx *ctx) { const struct cred *old_cred; @@ -306,18 +306,18 @@ ssize_t backing_file_splice_write(struct pipe_inode_info *pipe, if (!out->f_op->splice_write) return -EINVAL; - ret = file_remove_privs(ctx->user_file); + ret = file_remove_privs(iocb->ki_filp); if (ret) return ret; old_cred = override_creds(ctx->cred); file_start_write(out); - ret = out->f_op->splice_write(pipe, out, ppos, len, flags); + ret = out->f_op->splice_write(pipe, out, &iocb->ki_pos, len, flags); file_end_write(out); revert_creds(old_cred); if (ctx->end_write) - ctx->end_write(ctx->user_file, ppos ? *ppos : 0, ret); + ctx->end_write(iocb, ret); return ret; } @@ -329,8 +329,7 @@ int backing_file_mmap(struct file *file, struct vm_area_struct *vma, const struct cred *old_cred; int ret; - if (WARN_ON_ONCE(!(file->f_mode & FMODE_BACKING)) || - WARN_ON_ONCE(ctx->user_file != vma->vm_file)) + if (WARN_ON_ONCE(!(file->f_mode & FMODE_BACKING))) return -EIO; if (!file->f_op->mmap) @@ -343,7 +342,7 @@ int backing_file_mmap(struct file *file, struct vm_area_struct *vma, revert_creds(old_cred); if (ctx->accessed) - ctx->accessed(ctx->user_file); + ctx->accessed(vma->vm_file); return ret; } diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c index bbac547dfcb3c..607ef735ad4ab 100644 --- a/fs/fuse/passthrough.c +++ b/fs/fuse/passthrough.c @@ -18,11 +18,11 @@ static void fuse_file_accessed(struct file *file) fuse_invalidate_atime(inode); } -static void fuse_passthrough_end_write(struct file *file, loff_t pos, ssize_t ret) +static void fuse_passthrough_end_write(struct kiocb *iocb, ssize_t ret) { - struct inode *inode = file_inode(file); + struct inode *inode = file_inode(iocb->ki_filp); - fuse_write_update_attr(inode, pos, ret); + fuse_write_update_attr(inode, iocb->ki_pos, ret); } ssize_t fuse_passthrough_read_iter(struct kiocb *iocb, struct iov_iter *iter) @@ -34,7 +34,6 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb, struct iov_iter *iter) ssize_t ret; struct backing_file_ctx ctx = { .cred = ff->cred, - .user_file = file, .accessed = fuse_file_accessed, }; @@ -62,7 +61,6 @@ ssize_t fuse_passthrough_write_iter(struct kiocb *iocb, ssize_t ret; struct backing_file_ctx ctx = { .cred = ff->cred, - .user_file = file, .end_write = fuse_passthrough_end_write, }; @@ -88,15 +86,20 @@ ssize_t fuse_passthrough_splice_read(struct file *in, loff_t *ppos, struct file *backing_file = fuse_file_passthrough(ff); struct backing_file_ctx ctx = { .cred = ff->cred, - .user_file = in, .accessed = fuse_file_accessed, }; + struct kiocb iocb; + ssize_t ret; pr_debug("%s: backing_file=0x%p, pos=%lld, len=%zu, flags=0x%x\n", __func__, - backing_file, ppos ? *ppos : 0, len, flags); + backing_file, *ppos, len, flags); - return backing_file_splice_read(backing_file, ppos, pipe, len, flags, - &ctx); + init_sync_kiocb(&iocb, in); + iocb.ki_pos = *ppos; + ret = backing_file_splice_read(backing_file, &iocb, pipe, len, flags, &ctx); + *ppos = iocb.ki_pos; + + return ret; } ssize_t fuse_passthrough_splice_write(struct pipe_inode_info *pipe, @@ -109,16 +112,18 @@ ssize_t fuse_passthrough_splice_write(struct pipe_inode_info *pipe, ssize_t ret; struct backing_file_ctx ctx = { .cred = ff->cred, - .user_file = out, .end_write = fuse_passthrough_end_write, }; + struct kiocb iocb; pr_debug("%s: backing_file=0x%p, pos=%lld, len=%zu, flags=0x%x\n", __func__, - backing_file, ppos ? *ppos : 0, len, flags); + backing_file, *ppos, len, flags); inode_lock(inode); - ret = backing_file_splice_write(pipe, backing_file, ppos, len, flags, - &ctx); + init_sync_kiocb(&iocb, out); + iocb.ki_pos = *ppos; + ret = backing_file_splice_write(pipe, backing_file, &iocb, len, flags, &ctx); + *ppos = iocb.ki_pos; inode_unlock(inode); return ret; @@ -130,7 +135,6 @@ ssize_t fuse_passthrough_mmap(struct file *file, struct vm_area_struct *vma) struct file *backing_file = fuse_file_passthrough(ff); struct backing_file_ctx ctx = { .cred = ff->cred, - .user_file = file, .accessed = fuse_file_accessed, }; diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 4444c78e2e0c3..12c4d502ff916 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -231,9 +231,9 @@ static void ovl_file_modified(struct file *file) ovl_copyattr(file_inode(file)); } -static void ovl_file_end_write(struct file *file, loff_t pos, ssize_t ret) +static void ovl_file_end_write(struct kiocb *iocb, ssize_t ret) { - ovl_file_modified(file); + ovl_file_modified(iocb->ki_filp); } static void ovl_file_accessed(struct file *file) @@ -271,7 +271,6 @@ static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *iter) ssize_t ret; struct backing_file_ctx ctx = { .cred = ovl_creds(file_inode(file)->i_sb), - .user_file = file, .accessed = ovl_file_accessed, }; @@ -298,7 +297,6 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) int ifl = iocb->ki_flags; struct backing_file_ctx ctx = { .cred = ovl_creds(inode->i_sb), - .user_file = file, .end_write = ovl_file_end_write, }; @@ -338,15 +336,18 @@ static ssize_t ovl_splice_read(struct file *in, loff_t *ppos, ssize_t ret; struct backing_file_ctx ctx = { .cred = ovl_creds(file_inode(in)->i_sb), - .user_file = in, .accessed = ovl_file_accessed, }; + struct kiocb iocb; ret = ovl_real_fdget(in, &real); if (ret) return ret; - ret = backing_file_splice_read(fd_file(real), ppos, pipe, len, flags, &ctx); + init_sync_kiocb(&iocb, in); + iocb.ki_pos = *ppos; + ret = backing_file_splice_read(fd_file(real), &iocb, pipe, len, flags, &ctx); + *ppos = iocb.ki_pos; fdput(real); return ret; @@ -368,9 +369,9 @@ static ssize_t ovl_splice_write(struct pipe_inode_info *pipe, struct file *out, ssize_t ret; struct backing_file_ctx ctx = { .cred = ovl_creds(inode->i_sb), - .user_file = out, .end_write = ovl_file_end_write, }; + struct kiocb iocb; inode_lock(inode); /* Update mode */ @@ -380,9 +381,13 @@ static ssize_t ovl_splice_write(struct pipe_inode_info *pipe, struct file *out, if (ret) goto out_unlock; - ret = backing_file_splice_write(pipe, fd_file(real), ppos, len, flags, &ctx); + init_sync_kiocb(&iocb, out); + iocb.ki_pos = *ppos; + ret = backing_file_splice_write(pipe, fd_file(real), &iocb, len, flags, &ctx); + *ppos = iocb.ki_pos; fdput(real); + out_unlock: inode_unlock(inode); @@ -420,7 +425,6 @@ static int ovl_mmap(struct file *file, struct vm_area_struct *vma) struct file *realfile = file->private_data; struct backing_file_ctx ctx = { .cred = ovl_creds(file_inode(file)->i_sb), - .user_file = file, .accessed = ovl_file_accessed, }; diff --git a/include/linux/backing-file.h b/include/linux/backing-file.h index 2eed0ffb5e8f8..1476a6ed1bfd7 100644 --- a/include/linux/backing-file.h +++ b/include/linux/backing-file.h @@ -14,9 +14,8 @@ struct backing_file_ctx { const struct cred *cred; - struct file *user_file; - void (*accessed)(struct file *); - void (*end_write)(struct file *, loff_t, ssize_t); + void (*accessed)(struct file *file); + void (*end_write)(struct kiocb *iocb, ssize_t); }; struct file *backing_file_open(const struct path *user_path, int flags, @@ -31,13 +30,13 @@ ssize_t backing_file_read_iter(struct file *file, struct iov_iter *iter, ssize_t backing_file_write_iter(struct file *file, struct iov_iter *iter, struct kiocb *iocb, int flags, struct backing_file_ctx *ctx); -ssize_t backing_file_splice_read(struct file *in, loff_t *ppos, +ssize_t backing_file_splice_read(struct file *in, struct kiocb *iocb, struct pipe_inode_info *pipe, size_t len, unsigned int flags, struct backing_file_ctx *ctx); ssize_t backing_file_splice_write(struct pipe_inode_info *pipe, - struct file *out, loff_t *ppos, size_t len, - unsigned int flags, + struct file *out, struct kiocb *iocb, + size_t len, unsigned int flags, struct backing_file_ctx *ctx); int backing_file_mmap(struct file *file, struct vm_area_struct *vma, struct backing_file_ctx *ctx); From 49dffdfde462c7823de6ed882f71ce233aaeba63 Mon Sep 17 00:00:00 2001 From: Vinicius Costa Gomes Date: Wed, 6 Nov 2024 16:57:17 -0800 Subject: [PATCH 03/13] cred: Add a light version of override/revert_creds() Add a light version of override/revert_creds(), this should only be used when the credentials in question will outlive the critical section and the critical section doesn't change the ->usage of the credentials. Suggested-by: Christian Brauner Signed-off-by: Vinicius Costa Gomes Signed-off-by: Amir Goldstein --- include/linux/cred.h | 18 ++++++++++++++++++ kernel/cred.c | 6 +++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/include/linux/cred.h b/include/linux/cred.h index 2976f534a7a32..e4a3155fe409d 100644 --- a/include/linux/cred.h +++ b/include/linux/cred.h @@ -172,6 +172,24 @@ static inline bool cap_ambient_invariant_ok(const struct cred *cred) cred->cap_inheritable)); } +/* + * Override creds without bumping reference count. Caller must ensure + * reference remains valid or has taken reference. Almost always not the + * interface you want. Use override_creds()/revert_creds() instead. + */ +static inline const struct cred *override_creds_light(const struct cred *override_cred) +{ + const struct cred *old = current->cred; + + rcu_assign_pointer(current->cred, override_cred); + return old; +} + +static inline void revert_creds_light(const struct cred *revert_cred) +{ + rcu_assign_pointer(current->cred, revert_cred); +} + /** * get_new_cred_many - Get references on a new set of credentials * @cred: The new credentials to reference diff --git a/kernel/cred.c b/kernel/cred.c index 075cfa7c896f9..da7da250f7c8b 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -485,7 +485,7 @@ EXPORT_SYMBOL(abort_creds); */ const struct cred *override_creds(const struct cred *new) { - const struct cred *old = current->cred; + const struct cred *old; kdebug("override_creds(%p{%ld})", new, atomic_long_read(&new->usage)); @@ -499,7 +499,7 @@ const struct cred *override_creds(const struct cred *new) * visible to other threads under RCU. */ get_new_cred((struct cred *)new); - rcu_assign_pointer(current->cred, new); + old = override_creds_light(new); kdebug("override_creds() = %p{%ld}", old, atomic_long_read(&old->usage)); @@ -521,7 +521,7 @@ void revert_creds(const struct cred *old) kdebug("revert_creds(%p{%ld})", old, atomic_long_read(&old->usage)); - rcu_assign_pointer(current->cred, old); + revert_creds_light(old); put_cred(override); } EXPORT_SYMBOL(revert_creds); From d06ffd63a01226e9f741245ebac9b8f562fa04f5 Mon Sep 17 00:00:00 2001 From: Vinicius Costa Gomes Date: Wed, 6 Nov 2024 16:57:18 -0800 Subject: [PATCH 04/13] fs/backing-file: Convert to revert/override_creds_light() As the credentials used by backing-file are long lived in relation to the critical section (override_creds() -> revert_creds()) we can replace them by their lighter alternatives. Signed-off-by: Vinicius Costa Gomes Signed-off-by: Amir Goldstein --- fs/backing-file.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/fs/backing-file.c b/fs/backing-file.c index a38737592ec77..526ddb4d6f764 100644 --- a/fs/backing-file.c +++ b/fs/backing-file.c @@ -176,7 +176,7 @@ ssize_t backing_file_read_iter(struct file *file, struct iov_iter *iter, !(file->f_mode & FMODE_CAN_ODIRECT)) return -EINVAL; - old_cred = override_creds(ctx->cred); + old_cred = override_creds_light(ctx->cred); if (is_sync_kiocb(iocb)) { rwf_t rwf = iocb_to_rw_flags(flags); @@ -197,7 +197,7 @@ ssize_t backing_file_read_iter(struct file *file, struct iov_iter *iter, backing_aio_cleanup(aio, ret); } out: - revert_creds(old_cred); + revert_creds_light(old_cred); if (ctx->accessed) ctx->accessed(iocb->ki_filp); @@ -233,7 +233,7 @@ ssize_t backing_file_write_iter(struct file *file, struct iov_iter *iter, */ flags &= ~IOCB_DIO_CALLER_COMP; - old_cred = override_creds(ctx->cred); + old_cred = override_creds_light(ctx->cred); if (is_sync_kiocb(iocb)) { rwf_t rwf = iocb_to_rw_flags(flags); @@ -264,7 +264,7 @@ ssize_t backing_file_write_iter(struct file *file, struct iov_iter *iter, backing_aio_cleanup(aio, ret); } out: - revert_creds(old_cred); + revert_creds_light(old_cred); return ret; } @@ -281,9 +281,9 @@ ssize_t backing_file_splice_read(struct file *in, struct kiocb *iocb, if (WARN_ON_ONCE(!(in->f_mode & FMODE_BACKING))) return -EIO; - old_cred = override_creds(ctx->cred); + old_cred = override_creds_light(ctx->cred); ret = vfs_splice_read(in, &iocb->ki_pos, pipe, len, flags); - revert_creds(old_cred); + revert_creds_light(old_cred); if (ctx->accessed) ctx->accessed(iocb->ki_filp); @@ -310,11 +310,11 @@ ssize_t backing_file_splice_write(struct pipe_inode_info *pipe, if (ret) return ret; - old_cred = override_creds(ctx->cred); + old_cred = override_creds_light(ctx->cred); file_start_write(out); ret = out->f_op->splice_write(pipe, out, &iocb->ki_pos, len, flags); file_end_write(out); - revert_creds(old_cred); + revert_creds_light(old_cred); if (ctx->end_write) ctx->end_write(iocb, ret); @@ -337,9 +337,9 @@ int backing_file_mmap(struct file *file, struct vm_area_struct *vma, vma_set_file(vma, file); - old_cred = override_creds(ctx->cred); + old_cred = override_creds_light(ctx->cred); ret = call_mmap(vma->vm_file, vma); - revert_creds(old_cred); + revert_creds_light(old_cred); if (ctx->accessed) ctx->accessed(vma->vm_file); From fc5a1d2287bf23f67da1fc7a178cf26c5e6ba9d0 Mon Sep 17 00:00:00 2001 From: Vinicius Costa Gomes Date: Tue, 5 Nov 2024 11:35:13 -0800 Subject: [PATCH 05/13] ovl: use wrapper ovl_revert_creds() Introduce ovl_revert_creds() wrapper of revert_creds() to match callers of ovl_override_creds(). Suggested-by: Amir Goldstein Signed-off-by: Vinicius Costa Gomes Signed-off-by: Amir Goldstein --- fs/overlayfs/copy_up.c | 2 +- fs/overlayfs/dir.c | 10 +++++----- fs/overlayfs/file.c | 14 +++++++------- fs/overlayfs/inode.c | 20 ++++++++++---------- fs/overlayfs/namei.c | 10 +++++----- fs/overlayfs/overlayfs.h | 1 + fs/overlayfs/readdir.c | 8 ++++---- fs/overlayfs/util.c | 9 +++++++-- fs/overlayfs/xattrs.c | 9 ++++----- 9 files changed, 44 insertions(+), 39 deletions(-) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 2ed6ad641a206..dafd1c71b977e 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -1260,7 +1260,7 @@ static int ovl_copy_up_flags(struct dentry *dentry, int flags) dput(parent); dput(next); } - revert_creds(old_cred); + ovl_revert_creds(old_cred); return err; } diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index ab65e98a1defd..09db5eb19242f 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -621,7 +621,7 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, err = ovl_create_over_whiteout(dentry, inode, attr); out_revert_creds: - revert_creds(old_cred); + ovl_revert_creds(old_cred); return err; } @@ -702,7 +702,7 @@ static int ovl_set_link_redirect(struct dentry *dentry) old_cred = ovl_override_creds(dentry->d_sb); err = ovl_set_redirect(dentry, false); - revert_creds(old_cred); + ovl_revert_creds(old_cred); return err; } @@ -912,7 +912,7 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir) err = ovl_remove_upper(dentry, is_dir, &list); else err = ovl_remove_and_whiteout(dentry, &list); - revert_creds(old_cred); + ovl_revert_creds(old_cred); if (!err) { if (is_dir) clear_nlink(dentry->d_inode); @@ -1292,7 +1292,7 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, out_unlock: unlock_rename(new_upperdir, old_upperdir); out_revert_creds: - revert_creds(old_cred); + ovl_revert_creds(old_cred); if (update_nlink) ovl_nlink_end(new); else @@ -1337,7 +1337,7 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry, fput(realfile); } out_revert_creds: - revert_creds(old_cred); + ovl_revert_creds(old_cred); return err; } diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 12c4d502ff916..608a88ff8d813 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -51,7 +51,7 @@ static struct file *ovl_open_realfile(const struct file *file, realfile = backing_file_open(&file->f_path, flags, realpath, current_cred()); } - revert_creds(old_cred); + ovl_revert_creds(old_cred); pr_debug("open(%p[%pD2/%c], 0%o) -> (%p, 0%o)\n", file, file, ovl_whatisit(inode, realinode), file->f_flags, @@ -215,7 +215,7 @@ static loff_t ovl_llseek(struct file *file, loff_t offset, int whence) old_cred = ovl_override_creds(inode->i_sb); ret = vfs_llseek(fd_file(real), offset, whence); - revert_creds(old_cred); + ovl_revert_creds(old_cred); file->f_pos = fd_file(real)->f_pos; ovl_inode_unlock(inode); @@ -412,7 +412,7 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) if (file_inode(fd_file(real)) == ovl_inode_upper(file_inode(file))) { old_cred = ovl_override_creds(file_inode(file)->i_sb); ret = vfs_fsync_range(fd_file(real), start, end, datasync); - revert_creds(old_cred); + ovl_revert_creds(old_cred); } fdput(real); @@ -451,7 +451,7 @@ static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len old_cred = ovl_override_creds(file_inode(file)->i_sb); ret = vfs_fallocate(fd_file(real), mode, offset, len); - revert_creds(old_cred); + ovl_revert_creds(old_cred); /* Update size */ ovl_file_modified(file); @@ -476,7 +476,7 @@ static int ovl_fadvise(struct file *file, loff_t offset, loff_t len, int advice) old_cred = ovl_override_creds(file_inode(file)->i_sb); ret = vfs_fadvise(fd_file(real), offset, len, advice); - revert_creds(old_cred); + ovl_revert_creds(old_cred); fdput(real); @@ -535,7 +535,7 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in, flags); break; } - revert_creds(old_cred); + ovl_revert_creds(old_cred); /* Update size */ ovl_file_modified(file_out); @@ -597,7 +597,7 @@ static int ovl_flush(struct file *file, fl_owner_t id) if (fd_file(real)->f_op->flush) { old_cred = ovl_override_creds(file_inode(file)->i_sb); err = fd_file(real)->f_op->flush(fd_file(real), id); - revert_creds(old_cred); + ovl_revert_creds(old_cred); } fdput(real); diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index baa54c718bd72..a3798040532ab 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -80,7 +80,7 @@ int ovl_setattr(struct mnt_idmap *idmap, struct dentry *dentry, inode_lock(upperdentry->d_inode); old_cred = ovl_override_creds(dentry->d_sb); err = ovl_do_notify_change(ofs, upperdentry, attr); - revert_creds(old_cred); + ovl_revert_creds(old_cred); if (!err) ovl_copyattr(dentry->d_inode); inode_unlock(upperdentry->d_inode); @@ -280,7 +280,7 @@ int ovl_getattr(struct mnt_idmap *idmap, const struct path *path, stat->nlink = dentry->d_inode->i_nlink; out: - revert_creds(old_cred); + ovl_revert_creds(old_cred); return err; } @@ -317,7 +317,7 @@ int ovl_permission(struct mnt_idmap *idmap, mask |= MAY_READ; } err = inode_permission(mnt_idmap(realpath.mnt), realinode, mask); - revert_creds(old_cred); + ovl_revert_creds(old_cred); return err; } @@ -334,7 +334,7 @@ static const char *ovl_get_link(struct dentry *dentry, old_cred = ovl_override_creds(dentry->d_sb); p = vfs_get_link(ovl_dentry_real(dentry), done); - revert_creds(old_cred); + ovl_revert_creds(old_cred); return p; } @@ -469,7 +469,7 @@ struct posix_acl *do_ovl_get_acl(struct mnt_idmap *idmap, old_cred = ovl_override_creds(inode->i_sb); acl = ovl_get_acl_path(&realpath, posix_acl_xattr_name(type), noperm); - revert_creds(old_cred); + ovl_revert_creds(old_cred); } return acl; @@ -498,7 +498,7 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode, old_cred = ovl_override_creds(dentry->d_sb); real_acl = vfs_get_acl(mnt_idmap(realpath.mnt), realdentry, acl_name); - revert_creds(old_cred); + ovl_revert_creds(old_cred); if (IS_ERR(real_acl)) { err = PTR_ERR(real_acl); goto out; @@ -523,7 +523,7 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode, err = ovl_do_set_acl(ofs, realdentry, acl_name, acl); else err = ovl_do_remove_acl(ofs, realdentry, acl_name); - revert_creds(old_cred); + ovl_revert_creds(old_cred); ovl_drop_write(dentry); /* copy c/mtime */ @@ -600,7 +600,7 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, old_cred = ovl_override_creds(inode->i_sb); err = realinode->i_op->fiemap(realinode, fieinfo, start, len); - revert_creds(old_cred); + ovl_revert_creds(old_cred); return err; } @@ -676,7 +676,7 @@ int ovl_fileattr_set(struct mnt_idmap *idmap, err = ovl_set_protattr(inode, upperpath.dentry, fa); if (!err) err = ovl_real_fileattr_set(&upperpath, fa); - revert_creds(old_cred); + ovl_revert_creds(old_cred); ovl_drop_write(dentry); /* @@ -738,7 +738,7 @@ int ovl_fileattr_get(struct dentry *dentry, struct fileattr *fa) old_cred = ovl_override_creds(inode->i_sb); err = ovl_real_fileattr_get(&realpath, fa); ovl_fileattr_prot_flags(inode, fa); - revert_creds(old_cred); + ovl_revert_creds(old_cred); return err; } diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index 5764f91d283e7..7e27b7d4adee8 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -961,7 +961,7 @@ static int ovl_maybe_validate_verity(struct dentry *dentry) if (err == 0) ovl_set_flag(OVL_VERIFIED_DIGEST, inode); - revert_creds(old_cred); + ovl_revert_creds(old_cred); } ovl_inode_unlock(inode); @@ -995,7 +995,7 @@ static int ovl_maybe_lookup_lowerdata(struct dentry *dentry) old_cred = ovl_override_creds(dentry->d_sb); err = ovl_lookup_data_layers(dentry, redirect, &datapath); - revert_creds(old_cred); + ovl_revert_creds(old_cred); if (err) goto out_err; @@ -1342,7 +1342,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, ovl_dentry_init_reval(dentry, upperdentry, OVL_I_E(inode)); - revert_creds(old_cred); + ovl_revert_creds(old_cred); if (origin_path) { dput(origin_path->dentry); kfree(origin_path); @@ -1366,7 +1366,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, kfree(upperredirect); out: kfree(d.redirect); - revert_creds(old_cred); + ovl_revert_creds(old_cred); return ERR_PTR(err); } @@ -1423,7 +1423,7 @@ bool ovl_lower_positive(struct dentry *dentry) dput(this); } } - revert_creds(old_cred); + ovl_revert_creds(old_cred); return positive; } diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 0bfe35da4b7b7..7b7a6e3a43e2b 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -429,6 +429,7 @@ int ovl_want_write(struct dentry *dentry); void ovl_drop_write(struct dentry *dentry); struct dentry *ovl_workdir(struct dentry *dentry); const struct cred *ovl_override_creds(struct super_block *sb); +void ovl_revert_creds(const struct cred *old_cred); static inline const struct cred *ovl_creds(struct super_block *sb) { diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index 0ca8af060b0c1..881ec5592da52 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -290,7 +290,7 @@ static int ovl_check_whiteouts(const struct path *path, struct ovl_readdir_data } inode_unlock(dir->d_inode); } - revert_creds(old_cred); + ovl_revert_creds(old_cred); return err; } @@ -808,7 +808,7 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx) } err = 0; out: - revert_creds(old_cred); + ovl_revert_creds(old_cred); return err; } @@ -860,7 +860,7 @@ static struct file *ovl_dir_open_realfile(const struct file *file, old_cred = ovl_override_creds(file_inode(file)->i_sb); res = ovl_path_open(realpath, O_RDONLY | (file->f_flags & O_LARGEFILE)); - revert_creds(old_cred); + ovl_revert_creds(old_cred); return res; } @@ -987,7 +987,7 @@ int ovl_check_empty_dir(struct dentry *dentry, struct list_head *list) old_cred = ovl_override_creds(dentry->d_sb); err = ovl_dir_read_merged(dentry, list, &root); - revert_creds(old_cred); + ovl_revert_creds(old_cred); if (err) return err; diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index edc9216f6e27a..9408046f4f417 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -68,6 +68,11 @@ const struct cred *ovl_override_creds(struct super_block *sb) return override_creds(ofs->creator_cred); } +void ovl_revert_creds(const struct cred *old_cred) +{ + revert_creds(old_cred); +} + /* * Check if underlying fs supports file handles and try to determine encoding * type, in order to deduce maximum inode number used by fs. @@ -1178,7 +1183,7 @@ int ovl_nlink_start(struct dentry *dentry) * value relative to the upper inode nlink in an upper inode xattr. */ err = ovl_set_nlink_upper(dentry); - revert_creds(old_cred); + ovl_revert_creds(old_cred); if (err) goto out_drop_write; @@ -1203,7 +1208,7 @@ void ovl_nlink_end(struct dentry *dentry) old_cred = ovl_override_creds(dentry->d_sb); ovl_cleanup_index(dentry); - revert_creds(old_cred); + ovl_revert_creds(old_cred); } ovl_inode_unlock(inode); diff --git a/fs/overlayfs/xattrs.c b/fs/overlayfs/xattrs.c index 383978e4663c2..88055deca9360 100644 --- a/fs/overlayfs/xattrs.c +++ b/fs/overlayfs/xattrs.c @@ -47,7 +47,7 @@ static int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char ovl_path_lower(dentry, &realpath); old_cred = ovl_override_creds(dentry->d_sb); err = vfs_getxattr(mnt_idmap(realpath.mnt), realdentry, name, NULL, 0); - revert_creds(old_cred); + ovl_revert_creds(old_cred); if (err < 0) goto out; } @@ -72,7 +72,7 @@ static int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char WARN_ON(flags != XATTR_REPLACE); err = ovl_do_removexattr(ofs, realdentry, name); } - revert_creds(old_cred); + ovl_revert_creds(old_cred); ovl_drop_write(dentry); /* copy c/mtime */ @@ -91,7 +91,7 @@ static int ovl_xattr_get(struct dentry *dentry, struct inode *inode, const char ovl_i_path_real(inode, &realpath); old_cred = ovl_override_creds(dentry->d_sb); res = vfs_getxattr(mnt_idmap(realpath.mnt), realpath.dentry, name, value, size); - revert_creds(old_cred); + ovl_revert_creds(old_cred); return res; } @@ -121,7 +121,7 @@ ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size) old_cred = ovl_override_creds(dentry->d_sb); res = vfs_listxattr(realdentry, list, size); - revert_creds(old_cred); + ovl_revert_creds(old_cred); if (res <= 0 || size == 0) return res; @@ -268,4 +268,3 @@ const struct xattr_handler * const *ovl_xattr_handlers(struct ovl_fs *ofs) return ofs->config.userxattr ? ovl_user_xattr_handlers : ovl_trusted_xattr_handlers; } - From 711747e204ead127235aa41b350e518e92a87fcc Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Thu, 14 Nov 2024 09:36:35 +0100 Subject: [PATCH 06/13] ovl: pass an explicit reference of creators creds to callers ovl_setup_cred_for_create() decrements one refcount of new creds and ovl_revert_creds() in callers decrements the last refcount. In preparation to revert_creds_light() back to caller creds, pass an explicit reference of the creators creds to the callers and drop the refcount explicitly in the callers after ovl_revert_creds(). Reviewed-by: Christian Brauner Signed-off-by: Amir Goldstein --- fs/overlayfs/dir.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 09db5eb19242f..4b0bb7a91d376 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -553,15 +553,17 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, goto out_dput; } -static int ovl_setup_cred_for_create(struct dentry *dentry, struct inode *inode, - umode_t mode, const struct cred *old_cred) +static const struct cred *ovl_setup_cred_for_create(struct dentry *dentry, + struct inode *inode, + umode_t mode, + const struct cred *old_cred) { int err; struct cred *override_cred; override_cred = prepare_creds(); if (!override_cred) - return -ENOMEM; + return ERR_PTR(-ENOMEM); override_cred->fsuid = inode->i_uid; override_cred->fsgid = inode->i_gid; @@ -569,19 +571,18 @@ static int ovl_setup_cred_for_create(struct dentry *dentry, struct inode *inode, old_cred, override_cred); if (err) { put_cred(override_cred); - return err; + return ERR_PTR(err); } put_cred(override_creds(override_cred)); - put_cred(override_cred); - return 0; + return override_cred; } static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, struct ovl_cattr *attr, bool origin) { int err; - const struct cred *old_cred; + const struct cred *old_cred, *new_cred = NULL; struct dentry *parent = dentry->d_parent; old_cred = ovl_override_creds(dentry->d_sb); @@ -610,9 +611,13 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, * create a new inode, so just use the ovl mounter's * fs{u,g}id. */ - err = ovl_setup_cred_for_create(dentry, inode, attr->mode, old_cred); - if (err) + new_cred = ovl_setup_cred_for_create(dentry, inode, attr->mode, + old_cred); + err = PTR_ERR(new_cred); + if (IS_ERR(new_cred)) { + new_cred = NULL; goto out_revert_creds; + } } if (!ovl_dentry_is_whiteout(dentry)) @@ -622,6 +627,7 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, out_revert_creds: ovl_revert_creds(old_cred); + put_cred(new_cred); return err; } @@ -1306,7 +1312,7 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, static int ovl_create_tmpfile(struct file *file, struct dentry *dentry, struct inode *inode, umode_t mode) { - const struct cred *old_cred; + const struct cred *old_cred, *new_cred = NULL; struct path realparentpath; struct file *realfile; struct dentry *newdentry; @@ -1315,9 +1321,12 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry, int err; old_cred = ovl_override_creds(dentry->d_sb); - err = ovl_setup_cred_for_create(dentry, inode, mode, old_cred); - if (err) + new_cred = ovl_setup_cred_for_create(dentry, inode, mode, old_cred); + err = PTR_ERR(new_cred); + if (IS_ERR(new_cred)) { + new_cred = NULL; goto out_revert_creds; + } ovl_path_upper(dentry->d_parent, &realparentpath); realfile = backing_tmpfile_open(&file->f_path, flags, &realparentpath, @@ -1338,6 +1347,7 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry, } out_revert_creds: ovl_revert_creds(old_cred); + put_cred(new_cred); return err; } From c5b28fc161c5402da2e10cc11637c2dff727ac23 Mon Sep 17 00:00:00 2001 From: Vinicius Costa Gomes Date: Thu, 14 Nov 2024 09:52:23 +0100 Subject: [PATCH 07/13] ovl: Optimize override/revert creds Use override_creds_light() in ovl_override_creds() and revert_creds_light() in ovl_revert_creds(). The _light() functions do not change the 'usage' of the credentials in question, as they refer to the credentials associated with the mounter, which have a longer lifetime. In ovl_setup_cred_for_create(), do not need to modify the mounter credentials (returned by override_creds_light()) 'usage' counter. Add a warning to verify that we are indeed working with the mounter credentials (stored in the superblock). Failure in this assumption means that creds may leak. Suggested-by: Christian Brauner Signed-off-by: Vinicius Costa Gomes Signed-off-by: Amir Goldstein --- fs/overlayfs/dir.c | 10 +++++++++- fs/overlayfs/util.c | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 4b0bb7a91d376..5cf529482a8a3 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -573,7 +573,15 @@ static const struct cred *ovl_setup_cred_for_create(struct dentry *dentry, put_cred(override_cred); return ERR_PTR(err); } - put_cred(override_creds(override_cred)); + + /* + * Caller is going to match this with revert_creds_light() and drop + * referenec on the returned creds. + * We must be called with creator creds already, otherwise we risk + * leaking creds. + */ + old_cred = override_creds_light(override_cred); + WARN_ON_ONCE(old_cred != ovl_creds(dentry->d_sb)); return override_cred; } diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 9408046f4f417..3bb107471fb42 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -65,12 +65,12 @@ const struct cred *ovl_override_creds(struct super_block *sb) { struct ovl_fs *ofs = OVL_FS(sb); - return override_creds(ofs->creator_cred); + return override_creds_light(ofs->creator_cred); } void ovl_revert_creds(const struct cred *old_cred) { - revert_creds(old_cred); + revert_creds_light(old_cred); } /* From c2c54b5f34f6341f24d06689d4b986bd75b4b41c Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Tue, 5 Nov 2024 21:28:06 +0100 Subject: [PATCH 08/13] ovl: do not open non-data lower file for fsync ovl_fsync() with !datasync opens a backing file from the top most dentry in the stack, checks if this dentry is non-upper and skips the fsync. In case of an overlay dentry stack with lower data and lower metadata above it, but without an upper metadata above it, the backing file is opened from the top most lower metadata dentry and never used. Refactor the helper ovl_real_fdget_meta() into ovl_real_fdget_path() and open code the checks for non-upper inode in ovl_fsync(), so in that case we can avoid the unneeded backing file open. Signed-off-by: Amir Goldstein --- fs/overlayfs/file.c | 58 ++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 608a88ff8d813..4a1cf45e3fa14 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -89,32 +89,19 @@ static int ovl_change_flags(struct file *file, unsigned int flags) return 0; } -static int ovl_real_fdget_meta(const struct file *file, struct fd *real, - bool allow_meta) +static int ovl_real_fdget_path(const struct file *file, struct fd *real, + struct path *realpath) { - struct dentry *dentry = file_dentry(file); struct file *realfile = file->private_data; - struct path realpath; - int err; real->word = (unsigned long)realfile; - if (allow_meta) { - ovl_path_real(dentry, &realpath); - } else { - /* lazy lookup and verify of lowerdata */ - err = ovl_verify_lowerdata(dentry); - if (err) - return err; - - ovl_path_realdata(dentry, &realpath); - } - if (!realpath.dentry) + if (WARN_ON_ONCE(!realpath->dentry)) return -EIO; /* Has it been copied up since we'd opened it? */ - if (unlikely(file_inode(realfile) != d_inode(realpath.dentry))) { - struct file *f = ovl_open_realfile(file, &realpath); + if (unlikely(file_inode(realfile) != d_inode(realpath->dentry))) { + struct file *f = ovl_open_realfile(file, realpath); if (IS_ERR(f)) return PTR_ERR(f); real->word = (unsigned long)f | FDPUT_FPUT; @@ -130,7 +117,11 @@ static int ovl_real_fdget_meta(const struct file *file, struct fd *real, static int ovl_real_fdget(const struct file *file, struct fd *real) { - if (d_is_dir(file_dentry(file))) { + struct dentry *dentry = file_dentry(file); + struct path realpath; + int err; + + if (d_is_dir(dentry)) { struct file *f = ovl_dir_real_file(file, false); if (IS_ERR(f)) return PTR_ERR(f); @@ -138,7 +129,14 @@ static int ovl_real_fdget(const struct file *file, struct fd *real) return 0; } - return ovl_real_fdget_meta(file, real, false); + /* lazy lookup and verify of lowerdata */ + err = ovl_verify_lowerdata(dentry); + if (err) + return err; + + ovl_path_realdata(dentry, &realpath); + + return ovl_real_fdget_path(file, real, &realpath); } static int ovl_open(struct inode *inode, struct file *file) @@ -396,6 +394,9 @@ static ssize_t ovl_splice_write(struct pipe_inode_info *pipe, struct file *out, static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) { + struct dentry *dentry = file_dentry(file); + enum ovl_path_type type; + struct path upperpath; struct fd real; const struct cred *old_cred; int ret; @@ -404,16 +405,19 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) if (ret <= 0) return ret; - ret = ovl_real_fdget_meta(file, &real, !datasync); + /* Don't sync lower file for fear of receiving EROFS error */ + type = ovl_path_type(dentry); + if (!OVL_TYPE_UPPER(type) || (datasync && OVL_TYPE_MERGE(type))) + return 0; + + ovl_path_upper(dentry, &upperpath); + ret = ovl_real_fdget_path(file, &real, &upperpath); if (ret) return ret; - /* Don't sync lower file for fear of receiving EROFS error */ - if (file_inode(fd_file(real)) == ovl_inode_upper(file_inode(file))) { - old_cred = ovl_override_creds(file_inode(file)->i_sb); - ret = vfs_fsync_range(fd_file(real), start, end, datasync); - ovl_revert_creds(old_cred); - } + old_cred = ovl_override_creds(file_inode(file)->i_sb); + ret = vfs_fsync_range(fd_file(real), start, end, datasync); + ovl_revert_creds(old_cred); fdput(real); From 87a8a76c34a2ae6f667cc33249dc99705e363d1f Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Mon, 7 Oct 2024 15:22:29 +0200 Subject: [PATCH 09/13] ovl: allocate a container struct ovl_file for ovl private context Instead of using ->private_data to point at realfile directly, so that we can add more context per ovl open file. Signed-off-by: Amir Goldstein --- fs/overlayfs/dir.c | 14 +++++++++++--- fs/overlayfs/file.c | 40 ++++++++++++++++++++++++++++++++++------ fs/overlayfs/overlayfs.h | 3 +++ 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 5cf529482a8a3..08e683917d121 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -1323,6 +1323,7 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry, const struct cred *old_cred, *new_cred = NULL; struct path realparentpath; struct file *realfile; + struct ovl_file *of; struct dentry *newdentry; /* It's okay to set O_NOATIME, since the owner will be current fsuid */ int flags = file->f_flags | OVL_OPEN_FLAGS; @@ -1344,14 +1345,21 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry, if (err) goto out_revert_creds; + of = ovl_file_alloc(realfile); + if (!of) { + fput(realfile); + err = -ENOMEM; + goto out_revert_creds; + } + /* ovl_instantiate() consumes the newdentry reference on success */ newdentry = dget(realfile->f_path.dentry); err = ovl_instantiate(dentry, inode, newdentry, false, file); if (!err) { - file->private_data = realfile; + file->private_data = of; } else { dput(newdentry); - fput(realfile); + ovl_file_free(of); } out_revert_creds: ovl_revert_creds(old_cred); @@ -1407,7 +1415,7 @@ static int ovl_tmpfile(struct mnt_idmap *idmap, struct inode *dir, put_realfile: /* Without FMODE_OPENED ->release() won't be called on @file */ if (!(file->f_mode & FMODE_OPENED)) - fput(file->private_data); + ovl_file_free(file->private_data); put_inode: iput(inode); drop_write: diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 4a1cf45e3fa14..0fa0795d335f7 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -89,10 +89,32 @@ static int ovl_change_flags(struct file *file, unsigned int flags) return 0; } +struct ovl_file { + struct file *realfile; +}; + +struct ovl_file *ovl_file_alloc(struct file *realfile) +{ + struct ovl_file *of = kzalloc(sizeof(struct ovl_file), GFP_KERNEL); + + if (unlikely(!of)) + return NULL; + + of->realfile = realfile; + return of; +} + +void ovl_file_free(struct ovl_file *of) +{ + fput(of->realfile); + kfree(of); +} + static int ovl_real_fdget_path(const struct file *file, struct fd *real, struct path *realpath) { - struct file *realfile = file->private_data; + struct ovl_file *of = file->private_data; + struct file *realfile = of->realfile; real->word = (unsigned long)realfile; @@ -144,6 +166,7 @@ static int ovl_open(struct inode *inode, struct file *file) struct dentry *dentry = file_dentry(file); struct file *realfile; struct path realpath; + struct ovl_file *of; int err; /* lazy lookup and verify lowerdata */ @@ -166,15 +189,20 @@ static int ovl_open(struct inode *inode, struct file *file) if (IS_ERR(realfile)) return PTR_ERR(realfile); - file->private_data = realfile; + of = ovl_file_alloc(realfile); + if (!of) { + fput(realfile); + return -ENOMEM; + } + + file->private_data = of; return 0; } static int ovl_release(struct inode *inode, struct file *file) { - fput(file->private_data); - + ovl_file_free(file->private_data); return 0; } @@ -426,13 +454,13 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) static int ovl_mmap(struct file *file, struct vm_area_struct *vma) { - struct file *realfile = file->private_data; + struct ovl_file *of = file->private_data; struct backing_file_ctx ctx = { .cred = ovl_creds(file_inode(file)->i_sb), .accessed = ovl_file_accessed, }; - return backing_file_mmap(realfile, vma, &ctx); + return backing_file_mmap(of->realfile, vma, &ctx); } static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len) diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 7b7a6e3a43e2b..6e32eb9cd1b6e 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -863,6 +863,9 @@ int ovl_real_fileattr_set(const struct path *realpath, struct fileattr *fa); int ovl_fileattr_get(struct dentry *dentry, struct fileattr *fa); int ovl_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry, struct fileattr *fa); +struct ovl_file; +struct ovl_file *ovl_file_alloc(struct file *realfile); +void ovl_file_free(struct ovl_file *of); /* copy_up.c */ int ovl_copy_up(struct dentry *dentry); From 18e48d0e2c7b137be532fedbb5fba31b43e043b2 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Mon, 14 Oct 2024 17:25:26 +0200 Subject: [PATCH 10/13] ovl: store upper real file in ovl_file struct When an overlayfs file is opened as lower and then the file is copied up, every operation on the overlayfs open file will open a temporary backing file to the upper dentry and close it at the end of the operation. Store the upper real file along side the original (lower) real file in ovl_file instead of opening a temporary upper file on every operation. Signed-off-by: Amir Goldstein --- fs/overlayfs/file.c | 49 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 0fa0795d335f7..63804db4a0f4f 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -91,6 +91,7 @@ static int ovl_change_flags(struct file *file, unsigned int flags) struct ovl_file { struct file *realfile; + struct file *upperfile; }; struct ovl_file *ovl_file_alloc(struct file *realfile) @@ -107,33 +108,65 @@ struct ovl_file *ovl_file_alloc(struct file *realfile) void ovl_file_free(struct ovl_file *of) { fput(of->realfile); + if (of->upperfile) + fput(of->upperfile); kfree(of); } +static bool ovl_is_real_file(const struct file *realfile, + const struct path *realpath) +{ + return file_inode(realfile) == d_inode(realpath->dentry); +} + static int ovl_real_fdget_path(const struct file *file, struct fd *real, struct path *realpath) { struct ovl_file *of = file->private_data; struct file *realfile = of->realfile; - real->word = (unsigned long)realfile; + real->word = 0; if (WARN_ON_ONCE(!realpath->dentry)) return -EIO; - /* Has it been copied up since we'd opened it? */ - if (unlikely(file_inode(realfile) != d_inode(realpath->dentry))) { - struct file *f = ovl_open_realfile(file, realpath); - if (IS_ERR(f)) - return PTR_ERR(f); - real->word = (unsigned long)f | FDPUT_FPUT; - return 0; + /* + * If the realfile that we want is not where the data used to be at + * open time, either we'd been copied up, or it's an fsync of a + * metacopied file. We need the upperfile either way, so see if it + * is already opened and if it is not then open and store it. + */ + if (unlikely(!ovl_is_real_file(realfile, realpath))) { + struct file *upperfile = READ_ONCE(of->upperfile); + struct file *old; + + if (!upperfile) { /* Nobody opened upperfile yet */ + upperfile = ovl_open_realfile(file, realpath); + if (IS_ERR(upperfile)) + return PTR_ERR(upperfile); + + /* Store the upperfile for later */ + old = cmpxchg_release(&of->upperfile, NULL, upperfile); + if (old) { /* Someone opened upperfile before us */ + fput(upperfile); + upperfile = old; + } + } + /* + * Stored file must be from the right inode, unless someone's + * been corrupting the upper layer. + */ + if (WARN_ON_ONCE(!ovl_is_real_file(upperfile, realpath))) + return -EIO; + + realfile = upperfile; } /* Did the flags change since open? */ if (unlikely((file->f_flags ^ realfile->f_flags) & ~OVL_OPEN_FLAGS)) return ovl_change_flags(realfile, file->f_flags); + real->word = (unsigned long)realfile; return 0; } From 4333e42ed44448898a31665339702591284d1698 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Tue, 5 Nov 2024 21:28:49 +0100 Subject: [PATCH 11/13] ovl: convert ovl_real_fdget_path() callers to ovl_real_file_path() Stop using struct fd to return a real file from ovl_real_fdget_path(), because we no longer return a temporary file object and the callers always get a borrowed file reference. Rename the helper to ovl_real_file_path(), return a borrowed reference of the real file that is referenced from the overlayfs file or an error. Signed-off-by: Amir Goldstein --- fs/overlayfs/file.c | 59 ++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 63804db4a0f4f..a9c26a585b279 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -119,16 +119,14 @@ static bool ovl_is_real_file(const struct file *realfile, return file_inode(realfile) == d_inode(realpath->dentry); } -static int ovl_real_fdget_path(const struct file *file, struct fd *real, - struct path *realpath) +static struct file *ovl_real_file_path(const struct file *file, + struct path *realpath) { struct ovl_file *of = file->private_data; struct file *realfile = of->realfile; - real->word = 0; - if (WARN_ON_ONCE(!realpath->dentry)) - return -EIO; + return ERR_PTR(-EIO); /* * If the realfile that we want is not where the data used to be at @@ -143,7 +141,7 @@ static int ovl_real_fdget_path(const struct file *file, struct fd *real, if (!upperfile) { /* Nobody opened upperfile yet */ upperfile = ovl_open_realfile(file, realpath); if (IS_ERR(upperfile)) - return PTR_ERR(upperfile); + return upperfile; /* Store the upperfile for later */ old = cmpxchg_release(&of->upperfile, NULL, upperfile); @@ -157,20 +155,23 @@ static int ovl_real_fdget_path(const struct file *file, struct fd *real, * been corrupting the upper layer. */ if (WARN_ON_ONCE(!ovl_is_real_file(upperfile, realpath))) - return -EIO; + return ERR_PTR(-EIO); realfile = upperfile; } /* Did the flags change since open? */ - if (unlikely((file->f_flags ^ realfile->f_flags) & ~OVL_OPEN_FLAGS)) - return ovl_change_flags(realfile, file->f_flags); + if (unlikely((file->f_flags ^ realfile->f_flags) & ~OVL_OPEN_FLAGS)) { + int err = ovl_change_flags(realfile, file->f_flags); - real->word = (unsigned long)realfile; - return 0; + if (err) + return ERR_PTR(err); + } + + return realfile; } -static int ovl_real_fdget(const struct file *file, struct fd *real) +static struct file *ovl_real_file(const struct file *file) { struct dentry *dentry = file_dentry(file); struct path realpath; @@ -178,20 +179,30 @@ static int ovl_real_fdget(const struct file *file, struct fd *real) if (d_is_dir(dentry)) { struct file *f = ovl_dir_real_file(file, false); - if (IS_ERR(f)) - return PTR_ERR(f); - real->word = (unsigned long)f; - return 0; + + if (WARN_ON_ONCE(!f)) + return ERR_PTR(-EIO); + return f; } /* lazy lookup and verify of lowerdata */ err = ovl_verify_lowerdata(dentry); if (err) - return err; + return ERR_PTR(err); ovl_path_realdata(dentry, &realpath); - return ovl_real_fdget_path(file, real, &realpath); + return ovl_real_file_path(file, &realpath); +} + +static int ovl_real_fdget(const struct file *file, struct fd *real) +{ + struct file *f = ovl_real_file(file); + + if (IS_ERR(f)) + return PTR_ERR(f); + real->word = (unsigned long)f; + return 0; } static int ovl_open(struct inode *inode, struct file *file) @@ -458,7 +469,7 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) struct dentry *dentry = file_dentry(file); enum ovl_path_type type; struct path upperpath; - struct fd real; + struct file *upperfile; const struct cred *old_cred; int ret; @@ -472,16 +483,14 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) return 0; ovl_path_upper(dentry, &upperpath); - ret = ovl_real_fdget_path(file, &real, &upperpath); - if (ret) - return ret; + upperfile = ovl_real_file_path(file, &upperpath); + if (IS_ERR(upperfile)) + return PTR_ERR(upperfile); old_cred = ovl_override_creds(file_inode(file)->i_sb); - ret = vfs_fsync_range(fd_file(real), start, end, datasync); + ret = vfs_fsync_range(upperfile, start, end, datasync); ovl_revert_creds(old_cred); - fdput(real); - return ret; } From d66907b51ba07450bf9c6fb94364e3bf3f5e4c04 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Tue, 5 Nov 2024 21:29:36 +0100 Subject: [PATCH 12/13] ovl: convert ovl_real_fdget() callers to ovl_real_file() Stop using struct fd to return a real file from ovl_real_fdget(), because we no longer return a temporary file object and the callers always get a borrowed file reference. Rename the helper to ovl_real_file(), return a borrowed reference of the real file that is referenced from the overlayfs file or an error. Signed-off-by: Amir Goldstein --- fs/overlayfs/file.c | 143 ++++++++++++++++++-------------------------- 1 file changed, 59 insertions(+), 84 deletions(-) diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index a9c26a585b279..969b458100fe5 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -195,16 +195,6 @@ static struct file *ovl_real_file(const struct file *file) return ovl_real_file_path(file, &realpath); } -static int ovl_real_fdget(const struct file *file, struct fd *real) -{ - struct file *f = ovl_real_file(file); - - if (IS_ERR(f)) - return PTR_ERR(f); - real->word = (unsigned long)f; - return 0; -} - static int ovl_open(struct inode *inode, struct file *file) { struct dentry *dentry = file_dentry(file); @@ -253,7 +243,7 @@ static int ovl_release(struct inode *inode, struct file *file) static loff_t ovl_llseek(struct file *file, loff_t offset, int whence) { struct inode *inode = file_inode(file); - struct fd real; + struct file *realfile; const struct cred *old_cred; loff_t ret; @@ -269,9 +259,9 @@ static loff_t ovl_llseek(struct file *file, loff_t offset, int whence) return vfs_setpos(file, 0, 0); } - ret = ovl_real_fdget(file, &real); - if (ret) - return ret; + realfile = ovl_real_file(file); + if (IS_ERR(realfile)) + return PTR_ERR(realfile); /* * Overlay file f_pos is the master copy that is preserved @@ -281,17 +271,15 @@ static loff_t ovl_llseek(struct file *file, loff_t offset, int whence) * files, so we use the real file to perform seeks. */ ovl_inode_lock(inode); - fd_file(real)->f_pos = file->f_pos; + realfile->f_pos = file->f_pos; old_cred = ovl_override_creds(inode->i_sb); - ret = vfs_llseek(fd_file(real), offset, whence); + ret = vfs_llseek(realfile, offset, whence); ovl_revert_creds(old_cred); - file->f_pos = fd_file(real)->f_pos; + file->f_pos = realfile->f_pos; ovl_inode_unlock(inode); - fdput(real); - return ret; } @@ -337,8 +325,7 @@ static void ovl_file_accessed(struct file *file) static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *iter) { struct file *file = iocb->ki_filp; - struct fd real; - ssize_t ret; + struct file *realfile; struct backing_file_ctx ctx = { .cred = ovl_creds(file_inode(file)->i_sb), .accessed = ovl_file_accessed, @@ -347,22 +334,19 @@ static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *iter) if (!iov_iter_count(iter)) return 0; - ret = ovl_real_fdget(file, &real); - if (ret) - return ret; - - ret = backing_file_read_iter(fd_file(real), iter, iocb, iocb->ki_flags, - &ctx); - fdput(real); + realfile = ovl_real_file(file); + if (IS_ERR(realfile)) + return PTR_ERR(realfile); - return ret; + return backing_file_read_iter(realfile, iter, iocb, iocb->ki_flags, + &ctx); } static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) { struct file *file = iocb->ki_filp; struct inode *inode = file_inode(file); - struct fd real; + struct file *realfile; ssize_t ret; int ifl = iocb->ki_flags; struct backing_file_ctx ctx = { @@ -377,8 +361,9 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) /* Update mode */ ovl_copyattr(inode); - ret = ovl_real_fdget(file, &real); - if (ret) + realfile = ovl_real_file(file); + ret = PTR_ERR(realfile); + if (IS_ERR(realfile)) goto out_unlock; if (!ovl_should_sync(OVL_FS(inode->i_sb))) @@ -389,8 +374,7 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) * this property in case it is set by the issuer. */ ifl &= ~IOCB_DIO_CALLER_COMP; - ret = backing_file_write_iter(fd_file(real), iter, iocb, ifl, &ctx); - fdput(real); + ret = backing_file_write_iter(realfile, iter, iocb, ifl, &ctx); out_unlock: inode_unlock(inode); @@ -402,7 +386,7 @@ static ssize_t ovl_splice_read(struct file *in, loff_t *ppos, struct pipe_inode_info *pipe, size_t len, unsigned int flags) { - struct fd real; + struct file *realfile; ssize_t ret; struct backing_file_ctx ctx = { .cred = ovl_creds(file_inode(in)->i_sb), @@ -410,15 +394,14 @@ static ssize_t ovl_splice_read(struct file *in, loff_t *ppos, }; struct kiocb iocb; - ret = ovl_real_fdget(in, &real); - if (ret) - return ret; + realfile = ovl_real_file(in); + if (IS_ERR(realfile)) + return PTR_ERR(realfile); init_sync_kiocb(&iocb, in); iocb.ki_pos = *ppos; - ret = backing_file_splice_read(fd_file(real), &iocb, pipe, len, flags, &ctx); + ret = backing_file_splice_read(realfile, &iocb, pipe, len, flags, &ctx); *ppos = iocb.ki_pos; - fdput(real); return ret; } @@ -426,7 +409,7 @@ static ssize_t ovl_splice_read(struct file *in, loff_t *ppos, /* * Calling iter_file_splice_write() directly from overlay's f_op may deadlock * due to lock order inversion between pipe->mutex in iter_file_splice_write() - * and file_start_write(fd_file(real)) in ovl_write_iter(). + * and file_start_write(realfile) in ovl_write_iter(). * * So do everything ovl_write_iter() does and call iter_file_splice_write() on * the real file. @@ -434,7 +417,7 @@ static ssize_t ovl_splice_read(struct file *in, loff_t *ppos, static ssize_t ovl_splice_write(struct pipe_inode_info *pipe, struct file *out, loff_t *ppos, size_t len, unsigned int flags) { - struct fd real; + struct file *realfile; struct inode *inode = file_inode(out); ssize_t ret; struct backing_file_ctx ctx = { @@ -447,16 +430,15 @@ static ssize_t ovl_splice_write(struct pipe_inode_info *pipe, struct file *out, /* Update mode */ ovl_copyattr(inode); - ret = ovl_real_fdget(out, &real); - if (ret) + realfile = ovl_real_file(out); + ret = PTR_ERR(realfile); + if (IS_ERR(realfile)) goto out_unlock; init_sync_kiocb(&iocb, out); iocb.ki_pos = *ppos; - ret = backing_file_splice_write(pipe, fd_file(real), &iocb, len, flags, &ctx); + ret = backing_file_splice_write(pipe, realfile, &iocb, len, flags, &ctx); *ppos = iocb.ki_pos; - fdput(real); - out_unlock: inode_unlock(inode); @@ -508,7 +490,7 @@ static int ovl_mmap(struct file *file, struct vm_area_struct *vma) static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len) { struct inode *inode = file_inode(file); - struct fd real; + struct file *realfile; const struct cred *old_cred; int ret; @@ -519,19 +501,18 @@ static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len if (ret) goto out_unlock; - ret = ovl_real_fdget(file, &real); - if (ret) + realfile = ovl_real_file(file); + ret = PTR_ERR(realfile); + if (IS_ERR(realfile)) goto out_unlock; old_cred = ovl_override_creds(file_inode(file)->i_sb); - ret = vfs_fallocate(fd_file(real), mode, offset, len); + ret = vfs_fallocate(realfile, mode, offset, len); ovl_revert_creds(old_cred); /* Update size */ ovl_file_modified(file); - fdput(real); - out_unlock: inode_unlock(inode); @@ -540,20 +521,18 @@ static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len static int ovl_fadvise(struct file *file, loff_t offset, loff_t len, int advice) { - struct fd real; + struct file *realfile; const struct cred *old_cred; int ret; - ret = ovl_real_fdget(file, &real); - if (ret) - return ret; + realfile = ovl_real_file(file); + if (IS_ERR(realfile)) + return PTR_ERR(realfile); old_cred = ovl_override_creds(file_inode(file)->i_sb); - ret = vfs_fadvise(fd_file(real), offset, len, advice); + ret = vfs_fadvise(realfile, offset, len, advice); ovl_revert_creds(old_cred); - fdput(real); - return ret; } @@ -568,7 +547,7 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in, loff_t len, unsigned int flags, enum ovl_copyop op) { struct inode *inode_out = file_inode(file_out); - struct fd real_in, real_out; + struct file *realfile_in, *realfile_out; const struct cred *old_cred; loff_t ret; @@ -581,31 +560,31 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in, goto out_unlock; } - ret = ovl_real_fdget(file_out, &real_out); - if (ret) + realfile_out = ovl_real_file(file_out); + ret = PTR_ERR(realfile_out); + if (IS_ERR(realfile_out)) goto out_unlock; - ret = ovl_real_fdget(file_in, &real_in); - if (ret) { - fdput(real_out); + realfile_in = ovl_real_file(file_in); + ret = PTR_ERR(realfile_in); + if (IS_ERR(realfile_in)) goto out_unlock; - } old_cred = ovl_override_creds(file_inode(file_out)->i_sb); switch (op) { case OVL_COPY: - ret = vfs_copy_file_range(fd_file(real_in), pos_in, - fd_file(real_out), pos_out, len, flags); + ret = vfs_copy_file_range(realfile_in, pos_in, + realfile_out, pos_out, len, flags); break; case OVL_CLONE: - ret = vfs_clone_file_range(fd_file(real_in), pos_in, - fd_file(real_out), pos_out, len, flags); + ret = vfs_clone_file_range(realfile_in, pos_in, + realfile_out, pos_out, len, flags); break; case OVL_DEDUPE: - ret = vfs_dedupe_file_range_one(fd_file(real_in), pos_in, - fd_file(real_out), pos_out, len, + ret = vfs_dedupe_file_range_one(realfile_in, pos_in, + realfile_out, pos_out, len, flags); break; } @@ -614,9 +593,6 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in, /* Update size */ ovl_file_modified(file_out); - fdput(real_in); - fdput(real_out); - out_unlock: inode_unlock(inode_out); @@ -660,20 +636,19 @@ static loff_t ovl_remap_file_range(struct file *file_in, loff_t pos_in, static int ovl_flush(struct file *file, fl_owner_t id) { - struct fd real; + struct file *realfile; const struct cred *old_cred; - int err; + int err = 0; - err = ovl_real_fdget(file, &real); - if (err) - return err; + realfile = ovl_real_file(file); + if (IS_ERR(realfile)) + return PTR_ERR(realfile); - if (fd_file(real)->f_op->flush) { + if (realfile->f_op->flush) { old_cred = ovl_override_creds(file_inode(file)->i_sb); - err = fd_file(real)->f_op->flush(fd_file(real), id); + err = realfile->f_op->flush(realfile, id); ovl_revert_creds(old_cred); } - fdput(real); return err; } From c8b359dddb418c60df1a69beea01d1b3322bfe83 Mon Sep 17 00:00:00 2001 From: Vasiliy Kovalev Date: Tue, 19 Nov 2024 18:58:17 +0300 Subject: [PATCH 13/13] ovl: Filter invalid inodes with missing lookup function Add a check to the ovl_dentry_weird() function to prevent the processing of directory inodes that lack the lookup function. This is important because such inodes can cause errors in overlayfs when passed to the lowerstack. Reported-by: syzbot+a8c9d476508bd14a90e5@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?extid=a8c9d476508bd14a90e5 Suggested-by: Miklos Szeredi Link: https://lore.kernel.org/linux-unionfs/CAJfpegvx-oS9XGuwpJx=Xe28_jzWx5eRo1y900_ZzWY+=gGzUg@mail.gmail.com/ Signed-off-by: Vasiliy Kovalev Cc: Signed-off-by: Amir Goldstein --- fs/overlayfs/util.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 3bb107471fb42..9aa7493b1e103 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -202,6 +202,9 @@ void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry, bool ovl_dentry_weird(struct dentry *dentry) { + if (!d_can_lookup(dentry) && !d_is_file(dentry) && !d_is_symlink(dentry)) + return true; + return dentry->d_flags & (DCACHE_NEED_AUTOMOUNT | DCACHE_MANAGE_TRANSIT | DCACHE_OP_HASH |