From 941d935ac7636911a3fd8fa80e758e52b0b11e20 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Tue, 11 Jun 2019 18:09:28 +0300 Subject: ovl: fix wrong flags check in FS_IOC_FS[SG]ETXATTR ioctls The ioctl argument was parsed as the wrong type. Fixes: b21d9c435f93 ("ovl: support the FS_IOC_FS[SG]ETXATTR ioctls") Signed-off-by: Amir Goldstein Signed-off-by: Miklos Szeredi --- fs/overlayfs/file.c | 91 ++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 65 insertions(+), 26 deletions(-) diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 340a6ad45914..75d8d00fa087 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -409,37 +409,16 @@ static long ovl_real_ioctl(struct file *file, unsigned int cmd, return ret; } -static unsigned int ovl_get_inode_flags(struct inode *inode) -{ - unsigned int flags = READ_ONCE(inode->i_flags); - unsigned int ovl_iflags = 0; - - if (flags & S_SYNC) - ovl_iflags |= FS_SYNC_FL; - if (flags & S_APPEND) - ovl_iflags |= FS_APPEND_FL; - if (flags & S_IMMUTABLE) - ovl_iflags |= FS_IMMUTABLE_FL; - if (flags & S_NOATIME) - ovl_iflags |= FS_NOATIME_FL; - - return ovl_iflags; -} - static long ovl_ioctl_set_flags(struct file *file, unsigned int cmd, - unsigned long arg) + unsigned long arg, unsigned int iflags) { long ret; struct inode *inode = file_inode(file); - unsigned int flags; - unsigned int old_flags; + unsigned int old_iflags; if (!inode_owner_or_capable(inode)) return -EACCES; - if (get_user(flags, (int __user *) arg)) - return -EFAULT; - ret = mnt_want_write_file(file); if (ret) return ret; @@ -448,8 +427,8 @@ static long ovl_ioctl_set_flags(struct file *file, unsigned int cmd, /* Check the capability before cred override */ ret = -EPERM; - old_flags = ovl_get_inode_flags(inode); - if (((flags ^ old_flags) & (FS_APPEND_FL | FS_IMMUTABLE_FL)) && + old_iflags = READ_ONCE(inode->i_flags); + if (((iflags ^ old_iflags) & (S_APPEND | S_IMMUTABLE)) && !capable(CAP_LINUX_IMMUTABLE)) goto unlock; @@ -469,6 +448,63 @@ unlock: } +static unsigned int ovl_fsflags_to_iflags(unsigned int flags) +{ + unsigned int iflags = 0; + + if (flags & FS_SYNC_FL) + iflags |= S_SYNC; + if (flags & FS_APPEND_FL) + iflags |= S_APPEND; + if (flags & FS_IMMUTABLE_FL) + iflags |= S_IMMUTABLE; + if (flags & FS_NOATIME_FL) + iflags |= S_NOATIME; + + return iflags; +} + +static long ovl_ioctl_set_fsflags(struct file *file, unsigned int cmd, + unsigned long arg) +{ + unsigned int flags; + + if (get_user(flags, (int __user *) arg)) + return -EFAULT; + + return ovl_ioctl_set_flags(file, cmd, arg, + ovl_fsflags_to_iflags(flags)); +} + +static unsigned int ovl_fsxflags_to_iflags(unsigned int xflags) +{ + unsigned int iflags = 0; + + if (xflags & FS_XFLAG_SYNC) + iflags |= S_SYNC; + if (xflags & FS_XFLAG_APPEND) + iflags |= S_APPEND; + if (xflags & FS_XFLAG_IMMUTABLE) + iflags |= S_IMMUTABLE; + if (xflags & FS_XFLAG_NOATIME) + iflags |= S_NOATIME; + + return iflags; +} + +static long ovl_ioctl_set_fsxflags(struct file *file, unsigned int cmd, + unsigned long arg) +{ + struct fsxattr fa; + + memset(&fa, 0, sizeof(fa)); + if (copy_from_user(&fa, (void __user *) arg, sizeof(fa))) + return -EFAULT; + + return ovl_ioctl_set_flags(file, cmd, arg, + ovl_fsxflags_to_iflags(fa.fsx_xflags)); +} + static long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { long ret; @@ -480,8 +516,11 @@ static long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg) break; case FS_IOC_SETFLAGS: + ret = ovl_ioctl_set_fsflags(file, cmd, arg); + break; + case FS_IOC_FSSETXATTR: - ret = ovl_ioctl_set_flags(file, cmd, arg); + ret = ovl_ioctl_set_fsxflags(file, cmd, arg); break; default: -- cgit v1.2.3 From 9179c21dc6ed1c993caa5fe4da876a6765c26af7 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Tue, 18 Jun 2019 15:06:16 +0200 Subject: ovl: don't fail with disconnected lower NFS NFS mounts can be disconnected from fs root. Don't fail the overlapping layer check because of this. The check is not authoritative anyway, since topology can change during or after the check. Reported-by: Antti Antinoja Signed-off-by: Miklos Szeredi Fixes: 146d62e5a586 ("ovl: detect overlapping layers") --- fs/overlayfs/super.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 746ea36f3171..9b18e0f9016a 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -1471,23 +1471,20 @@ out_err: * Check if this layer root is a descendant of: * - another layer of this overlayfs instance * - upper/work dir of any overlayfs instance - * - a disconnected dentry (detached root) */ static int ovl_check_layer(struct super_block *sb, struct dentry *dentry, const char *name) { - struct dentry *next, *parent; - bool is_root = false; + struct dentry *next = dentry, *parent; int err = 0; - if (!dentry || dentry == dentry->d_sb->s_root) + if (!dentry) return 0; - next = dget(dentry); - /* Walk back ancestors to fs root (inclusive) looking for traps */ - do { - parent = dget_parent(next); - is_root = (parent == next); + parent = dget_parent(next); + + /* Walk back ancestors to root (inclusive) looking for traps */ + while (!err && parent != next) { if (ovl_is_inuse(parent)) { err = -EBUSY; pr_err("overlayfs: %s path overlapping in-use upperdir/workdir\n", @@ -1496,17 +1493,12 @@ static int ovl_check_layer(struct super_block *sb, struct dentry *dentry, err = -ELOOP; pr_err("overlayfs: overlapping %s path\n", name); } - dput(next); next = parent; - } while (!err && !is_root); - - /* Did we really walk to fs root or found a detached root? */ - if (!err && next != dentry->d_sb->s_root) { - err = -ESTALE; - pr_err("overlayfs: disconnected %s path\n", name); + parent = dget_parent(next); + dput(next); } - dput(next); + dput(parent); return err; } -- cgit v1.2.3 From 1dac6f5b0ed2601be21bb4e27a44b0c3e667b7f4 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Mon, 17 Jun 2019 14:39:29 +0200 Subject: ovl: fix bogus -Wmaybe-unitialized warning gcc gets a bit confused by the logic in ovl_setup_trap() and can't figure out whether the local 'trap' variable in the caller was initialized or not: fs/overlayfs/super.c: In function 'ovl_fill_super': fs/overlayfs/super.c:1333:4: error: 'trap' may be used uninitialized in this function [-Werror=maybe-uninitialized] iput(trap); ^~~~~~~~~~ fs/overlayfs/super.c:1312:17: note: 'trap' was declared here Reword slightly to make it easier for the compiler to understand. Fixes: 146d62e5a586 ("ovl: detect overlapping layers") Signed-off-by: Arnd Bergmann Signed-off-by: Miklos Szeredi --- fs/overlayfs/super.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 9b18e0f9016a..69888df10c90 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -995,8 +995,8 @@ static int ovl_setup_trap(struct super_block *sb, struct dentry *dir, int err; trap = ovl_get_trap_inode(sb, dir); - err = PTR_ERR(trap); - if (IS_ERR(trap)) { + err = PTR_ERR_OR_ZERO(trap); + if (err) { if (err == -ELOOP) pr_err("overlayfs: conflicting %s path\n", name); return err; -- cgit v1.2.3 From 253e7483391190c93696bc9710a31b2db9b63d77 Mon Sep 17 00:00:00 2001 From: Nicolas Schier Date: Mon, 17 Jun 2019 09:39:00 +0200 Subject: ovl: fix typo in MODULE_PARM_DESC Change first argument to MODULE_PARM_DESC() calls, that each of them matched the actual module parameter name. The matching results in changing (the 'parm' section from) the output of `modinfo overlay` from: parm: ovl_check_copy_up:Obsolete; does nothing parm: redirect_max:ushort parm: ovl_redirect_max:Maximum length of absolute redirect xattr value parm: redirect_dir:bool parm: ovl_redirect_dir_def:Default to on or off for the redirect_dir feature parm: redirect_always_follow:bool parm: ovl_redirect_always_follow:Follow redirects even if redirect_dir feature is turned off parm: index:bool parm: ovl_index_def:Default to on or off for the inodes index feature parm: nfs_export:bool parm: ovl_nfs_export_def:Default to on or off for the NFS export feature parm: xino_auto:bool parm: ovl_xino_auto_def:Auto enable xino feature parm: metacopy:bool parm: ovl_metacopy_def:Default to on or off for the metadata only copy up feature into: parm: check_copy_up:Obsolete; does nothing parm: redirect_max:Maximum length of absolute redirect xattr value (ushort) parm: redirect_dir:Default to on or off for the redirect_dir feature (bool) parm: redirect_always_follow:Follow redirects even if redirect_dir feature is turned off (bool) parm: index:Default to on or off for the inodes index feature (bool) parm: nfs_export:Default to on or off for the NFS export feature (bool) parm: xino_auto:Auto enable xino feature (bool) parm: metacopy:Default to on or off for the metadata only copy up feature (bool) Signed-off-by: Nicolas Schier Signed-off-by: Miklos Szeredi --- fs/overlayfs/copy_up.c | 2 +- fs/overlayfs/dir.c | 2 +- fs/overlayfs/super.c | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 56feaa739979..7ec3acd90d0b 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -37,7 +37,7 @@ static int ovl_ccup_get(char *buf, const struct kernel_param *param) } module_param_call(check_copy_up, ovl_ccup_set, ovl_ccup_get, NULL, 0644); -MODULE_PARM_DESC(ovl_check_copy_up, "Obsolete; does nothing"); +MODULE_PARM_DESC(check_copy_up, "Obsolete; does nothing"); int ovl_copy_xattr(struct dentry *old, struct dentry *new) { diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 93872bb50230..2f39046ebfa4 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -21,7 +21,7 @@ static unsigned short ovl_redirect_max = 256; module_param_named(redirect_max, ovl_redirect_max, ushort, 0644); -MODULE_PARM_DESC(ovl_redirect_max, +MODULE_PARM_DESC(redirect_max, "Maximum length of absolute redirect xattr value"); static int ovl_set_redirect(struct dentry *dentry, bool samedir); diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 69888df10c90..e300c1371b7b 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -31,29 +31,29 @@ struct ovl_dir_cache; static bool ovl_redirect_dir_def = IS_ENABLED(CONFIG_OVERLAY_FS_REDIRECT_DIR); module_param_named(redirect_dir, ovl_redirect_dir_def, bool, 0644); -MODULE_PARM_DESC(ovl_redirect_dir_def, +MODULE_PARM_DESC(redirect_dir, "Default to on or off for the redirect_dir feature"); static bool ovl_redirect_always_follow = IS_ENABLED(CONFIG_OVERLAY_FS_REDIRECT_ALWAYS_FOLLOW); module_param_named(redirect_always_follow, ovl_redirect_always_follow, bool, 0644); -MODULE_PARM_DESC(ovl_redirect_always_follow, +MODULE_PARM_DESC(redirect_always_follow, "Follow redirects even if redirect_dir feature is turned off"); static bool ovl_index_def = IS_ENABLED(CONFIG_OVERLAY_FS_INDEX); module_param_named(index, ovl_index_def, bool, 0644); -MODULE_PARM_DESC(ovl_index_def, +MODULE_PARM_DESC(index, "Default to on or off for the inodes index feature"); static bool ovl_nfs_export_def = IS_ENABLED(CONFIG_OVERLAY_FS_NFS_EXPORT); module_param_named(nfs_export, ovl_nfs_export_def, bool, 0644); -MODULE_PARM_DESC(ovl_nfs_export_def, +MODULE_PARM_DESC(nfs_export, "Default to on or off for the NFS export feature"); static bool ovl_xino_auto_def = IS_ENABLED(CONFIG_OVERLAY_FS_XINO_AUTO); module_param_named(xino_auto, ovl_xino_auto_def, bool, 0644); -MODULE_PARM_DESC(ovl_xino_auto_def, +MODULE_PARM_DESC(xino_auto, "Auto enable xino feature"); static void ovl_entry_stack_free(struct ovl_entry *oe) @@ -66,7 +66,7 @@ static void ovl_entry_stack_free(struct ovl_entry *oe) static bool ovl_metacopy_def = IS_ENABLED(CONFIG_OVERLAY_FS_METACOPY); module_param_named(metacopy, ovl_metacopy_def, bool, 0644); -MODULE_PARM_DESC(ovl_metacopy_def, +MODULE_PARM_DESC(metacopy, "Default to on or off for the metadata only copy up feature"); static void ovl_dentry_release(struct dentry *dentry) -- cgit v1.2.3 From 6dde1e42f497b2d4e22466f23019016775607947 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sun, 9 Jun 2019 19:03:44 +0300 Subject: ovl: make i_ino consistent with st_ino in more cases Relax the condition that overlayfs supports nfs export, to require that i_ino is consistent with st_ino/d_ino. It is enough to require that st_ino and d_ino are consistent. This fixes the failure of xfstest generic/504, due to mismatch of st_ino to inode number in the output of /proc/locks. Fixes: 12574a9f4c9c ("ovl: consistent i_ino for non-samefs with xino") Cc: # v4.19 Signed-off-by: Amir Goldstein Signed-off-by: Miklos Szeredi --- fs/overlayfs/inode.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index f7eba21effa5..f0389849fd80 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -553,15 +553,15 @@ static void ovl_fill_inode(struct inode *inode, umode_t mode, dev_t rdev, int xinobits = ovl_xino_bits(inode->i_sb); /* - * When NFS export is enabled and d_ino is consistent with st_ino - * (samefs or i_ino has enough bits to encode layer), set the same - * value used for d_ino to i_ino, because nfsd readdirplus compares - * d_ino values to i_ino values of child entries. When called from + * When d_ino is consistent with st_ino (samefs or i_ino has enough + * bits to encode layer), set the same value used for st_ino to i_ino, + * so inode number exposed via /proc/locks and a like will be + * consistent with d_ino and st_ino values. An i_ino value inconsistent + * with d_ino also causes nfsd readdirplus to fail. When called from * ovl_new_inode(), ino arg is 0, so i_ino will be updated to real * upper inode i_ino on ovl_inode_init() or ovl_inode_update(). */ - if (inode->i_sb->s_export_op && - (ovl_same_sb(inode->i_sb) || xinobits)) { + if (ovl_same_sb(inode->i_sb) || xinobits) { inode->i_ino = ino; if (xinobits && fsid && !(ino >> (64 - xinobits))) inode->i_ino |= (unsigned long)fsid << (64 - xinobits); -- cgit v1.2.3