From: Miklos Szeredi mszeredi@redhat.com
commit 05acefb4872dae89e772729efb194af754c877e8 upstream.
Call inode_permission() on real inode before opening regular file on one of the underlying layers.
In some cases ovl_permission() already checks access to an underlying file, but it misses the metacopy case, and possibly other ones as well.
Removing the redundant permission check from ovl_permission() should be considered later.
Signed-off-by: Miklos Szeredi mszeredi@redhat.com Signed-off-by: Hugo SIMELIERE hsimeliere.opensource@witekio.com --- fs/overlayfs/file.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 73c3e2c21edb..24a83ed9829a 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -34,10 +34,22 @@ static struct file *ovl_open_realfile(const struct file *file, struct file *realfile; const struct cred *old_cred; int flags = file->f_flags | OVL_OPEN_FLAGS; + int acc_mode = ACC_MODE(flags); + int err; + + if (flags & O_APPEND) + acc_mode |= MAY_APPEND;
old_cred = ovl_override_creds(inode->i_sb); - realfile = open_with_fake_path(&file->f_path, flags, realinode, - current_cred()); + err = inode_permission(realinode, MAY_OPEN | acc_mode); + if (err) { + realfile = ERR_PTR(err); + } else if (!inode_owner_or_capable(realinode)) { + realfile = ERR_PTR(-EPERM); + } else { + realfile = open_with_fake_path(&file->f_path, flags, realinode, + current_cred()); + } revert_creds(old_cred);
pr_debug("open(%p[%pD2/%c], 0%o) -> (%p, 0%o)\n",
From: Miklos Szeredi mszeredi@redhat.com
commit 56230d956739b9cb1cbde439d76227d77979a04d upstream.
Check permission before opening a real file.
ovl_path_open() is used by readdir and copy-up routines.
ovl_permission() theoretically already checked copy up permissions, but it doesn't hurt to re-do these checks during the actual copy-up.
For directory reading ovl_permission() only checks access to topmost underlying layer. Readdir on a merged directory accesses layers below the topmost one as well. Permission wasn't checked for these layers.
Note: modifying ovl_permission() to perform this check would be far more complex and hence more bug prone. The result is less precise permissions returned in access(2). If this turns out to be an issue, we can revisit this bug.
Signed-off-by: Miklos Szeredi mszeredi@redhat.com Signed-off-by: Hugo SIMELIERE hsimeliere.opensource@witekio.com --- fs/overlayfs/util.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index db8bdb29b320..afbc6a97da2a 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -479,7 +479,32 @@ bool ovl_is_whiteout(struct dentry *dentry)
struct file *ovl_path_open(struct path *path, int flags) { - return dentry_open(path, flags | O_NOATIME, current_cred()); + struct inode *inode = d_inode(path->dentry); + int err, acc_mode; + + if (flags & ~(O_ACCMODE | O_LARGEFILE)) + BUG(); + + switch (flags & O_ACCMODE) { + case O_RDONLY: + acc_mode = MAY_READ; + break; + case O_WRONLY: + acc_mode = MAY_WRITE; + break; + default: + BUG(); + } + + err = inode_permission(inode, acc_mode | MAY_OPEN); + if (err) + return ERR_PTR(err); + + /* O_NOATIME is an optimization, don't fail if not permitted */ + if (inode_owner_or_capable(inode)) + flags |= O_NOATIME; + + return dentry_open(path, flags, current_cred()); }
/* Caller should hold ovl_inode->lock */
From: Miklos Szeredi mszeredi@redhat.com
commit 48bd024b8a40d73ad6b086de2615738da0c7004f upstream.
In preparation for more permission checking, override credentials for directory operations on the underlying filesystems.
Signed-off-by: Miklos Szeredi mszeredi@redhat.com Signed-off-by: Hugo SIMELIERE hsimeliere.opensource@witekio.com --- fs/overlayfs/readdir.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index 11b7941c5dbc..db9132a020de 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -735,8 +735,10 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx) struct ovl_dir_file *od = file->private_data; struct dentry *dentry = file->f_path.dentry; struct ovl_cache_entry *p; + const struct cred *old_cred; int err;
+ old_cred = ovl_override_creds(dentry->d_sb); if (!ctx->pos) ovl_dir_reset(file);
@@ -750,17 +752,20 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx) (ovl_same_sb(dentry->d_sb) && (ovl_is_impure_dir(file) || OVL_TYPE_MERGE(ovl_path_type(dentry->d_parent))))) { - return ovl_iterate_real(file, ctx); + err = ovl_iterate_real(file, ctx); + } else { + err = iterate_dir(od->realfile, ctx); } - return iterate_dir(od->realfile, ctx); + goto out; }
if (!od->cache) { struct ovl_dir_cache *cache;
cache = ovl_cache_get(dentry); + err = PTR_ERR(cache); if (IS_ERR(cache)) - return PTR_ERR(cache); + goto out;
od->cache = cache; ovl_seek_cursor(od, ctx->pos); @@ -772,7 +777,7 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx) if (!p->ino) { err = ovl_cache_update_ino(&file->f_path, p); if (err) - return err; + goto out; } if (!dir_emit(ctx, p->name, p->len, p->ino, p->type)) break; @@ -780,7 +785,10 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx) od->cursor = p->l_node.next; ctx->pos++; } - return 0; + err = 0; +out: + revert_creds(old_cred); + return err; }
static loff_t ovl_dir_llseek(struct file *file, loff_t offset, int origin)
From: Miklos Szeredi mszeredi@redhat.com
commit b6650dab404c701d7fe08a108b746542a934da84 upstream.
In case the file cannot be opened with O_NOATIME because of lack of capabilities, then clear O_NOATIME instead of failing.
Remove WARN_ON(), since it would now trigger if O_NOATIME was cleared. Noticed by Amir Goldstein.
Signed-off-by: Miklos Szeredi mszeredi@redhat.com Signed-off-by: Hugo SIMELIERE hsimeliere.opensource@witekio.com --- fs/overlayfs/file.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 24a83ed9829a..65a7c600f228 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -44,9 +44,10 @@ static struct file *ovl_open_realfile(const struct file *file, err = inode_permission(realinode, MAY_OPEN | acc_mode); if (err) { realfile = ERR_PTR(err); - } else if (!inode_owner_or_capable(realinode)) { - realfile = ERR_PTR(-EPERM); } else { + if (!inode_owner_or_capable(realinode)) + flags &= ~O_NOATIME; + realfile = open_with_fake_path(&file->f_path, flags, realinode, current_cred()); } @@ -66,12 +67,6 @@ static int ovl_change_flags(struct file *file, unsigned int flags) struct inode *inode = file_inode(file); int err;
- flags |= OVL_OPEN_FLAGS; - - /* If some flag changed that cannot be changed then something's amiss */ - if (WARN_ON((file->f_flags ^ flags) & ~OVL_SETFL_MASK)) - return -EIO; - flags &= OVL_SETFL_MASK;
if (((flags ^ file->f_flags) & O_APPEND) && IS_APPEND(inode))
From: Miklos Szeredi mszeredi@redhat.com
commit 130fdbc3d1f9966dd4230709c30f3768bccd3065 upstream.
The three instances of ovl_path_open() in overlayfs/readdir.c do three different things:
- pass f_flags from overlay file - pass O_RDONLY | O_DIRECTORY - pass just O_RDONLY
The value of f_flags can be (other than O_RDONLY):
O_WRONLY - not possible for a directory O_RDWR - not possible for a directory O_CREAT - masked out by dentry_open() O_EXCL - masked out by dentry_open() O_NOCTTY - masked out by dentry_open() O_TRUNC - masked out by dentry_open() O_APPEND - no effect on directory ops O_NDELAY - no effect on directory ops O_NONBLOCK - no effect on directory ops __O_SYNC - no effect on directory ops O_DSYNC - no effect on directory ops FASYNC - no effect on directory ops O_DIRECT - no effect on directory ops O_LARGEFILE - ? O_DIRECTORY - only affects lookup O_NOFOLLOW - only affects lookup O_NOATIME - overlay sets this unconditionally in ovl_path_open() O_CLOEXEC - only affects fd allocation O_PATH - no effect on directory ops __O_TMPFILE - not possible for a directory
Fon non-merge directories we use the underlying filesystem's iterate; in this case honor O_LARGEFILE from the original file to make sure that open doesn't get rejected.
For merge directories it's safe to pass O_LARGEFILE unconditionally since userspace will only see the artificial offsets created by overlayfs.
Signed-off-by: Miklos Szeredi mszeredi@redhat.com Signed-off-by: Hugo SIMELIERE hsimeliere.opensource@witekio.com --- fs/overlayfs/readdir.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index db9132a020de..2e12b756cc82 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -300,7 +300,7 @@ static inline int ovl_dir_read(struct path *realpath, struct file *realfile; int err;
- realfile = ovl_path_open(realpath, O_RDONLY | O_DIRECTORY); + realfile = ovl_path_open(realpath, O_RDONLY | O_LARGEFILE); if (IS_ERR(realfile)) return PTR_ERR(realfile);
@@ -831,6 +831,12 @@ static loff_t ovl_dir_llseek(struct file *file, loff_t offset, int origin) return res; }
+static struct file *ovl_dir_open_realfile(struct file *file, + struct path *realpath) +{ + return ovl_path_open(realpath, O_RDONLY | (file->f_flags & O_LARGEFILE)); +} + static int ovl_dir_fsync(struct file *file, loff_t start, loff_t end, int datasync) { @@ -853,7 +859,7 @@ static int ovl_dir_fsync(struct file *file, loff_t start, loff_t end, struct path upperpath;
ovl_path_upper(dentry, &upperpath); - realfile = ovl_path_open(&upperpath, O_RDONLY); + realfile = ovl_dir_open_realfile(file, &upperpath);
inode_lock(inode); if (!od->upperfile) { @@ -904,7 +910,7 @@ static int ovl_dir_open(struct inode *inode, struct file *file) return -ENOMEM;
type = ovl_path_real(file->f_path.dentry, &realpath); - realfile = ovl_path_open(&realpath, file->f_flags); + realfile = ovl_dir_open_realfile(file, &realpath); if (IS_ERR(realfile)) { kfree(od); return PTR_ERR(realfile);
From: Miklos Szeredi mszeredi@redhat.com
commit 292f902a40c11f043a5ca1305a114da0e523eaa3 upstream.
Verify LSM permissions for underlying file, since vfs_ioctl() doesn't do it.
[Stephen Rothwell] export security_file_ioctl
Signed-off-by: Miklos Szeredi mszeredi@redhat.com Signed-off-by: Hugo SIMELIERE hsimeliere.opensource@witekio.com --- fs/overlayfs/file.c | 5 ++++- security/security.c | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 65a7c600f228..470ea215bebc 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -12,6 +12,7 @@ #include <linux/xattr.h> #include <linux/uio.h> #include <linux/uaccess.h> +#include <linux/security.h> #include "overlayfs.h"
static char ovl_whatisit(struct inode *inode, struct inode *realinode) @@ -410,7 +411,9 @@ static long ovl_real_ioctl(struct file *file, unsigned int cmd, return ret;
old_cred = ovl_override_creds(file_inode(file)->i_sb); - ret = vfs_ioctl(real.file, cmd, arg); + ret = security_file_ioctl(real.file, cmd, arg); + if (!ret) + ret = vfs_ioctl(real.file, cmd, arg); revert_creds(old_cred);
fdput(real); diff --git a/security/security.c b/security/security.c index e8a53164e6b5..035d8d87d07b 100644 --- a/security/security.c +++ b/security/security.c @@ -889,6 +889,7 @@ int security_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { return call_int_hook(file_ioctl, 0, file, cmd, arg); } +EXPORT_SYMBOL_GPL(security_file_ioctl);
/** * security_file_ioctl_compat() - Check if an ioctl is allowed in compat mode
On Thu, Aug 29, 2024 at 05:17:00PM +0200, hsimeliere.opensource@witekio.com wrote:
From: Miklos Szeredi mszeredi@redhat.com
commit 05acefb4872dae89e772729efb194af754c877e8 upstream.
<snip>
For obvious reasons we can not take a backport to an older kernel tree and not all newer ones as well, otherwise you would upgrade and have a regression. I think it says that in the documentation as well somewhere...
Please create patch series for all currently maintained kernel trees if you wish to have stuff like this merged, and resend them.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
hsimeliere.opensource＠witekio.com