Skip to contentSkip to conversation

ovl: also update attr on ovl level inode in ovl_setattr()

PATCH New Eryu Guan 1 participant 2 messages Dec 11, 2015 Overlay filesystem

lore
Share & tools
lore

Eryu Guan

author
#0 lore raw

PATCH 1/2 ovl: also update attr on ovl level inode in ovl_setattr()

suid and sgid should be cleared when unprivileged user writes to such files. Overlayfs fails to do so because of the ovl level inode has no S_ISUID or S_ISGID set and should_remove_suid() fails to set ATTR_KILL_[SG]ID. xfstests generic/193 reveals the failure on overlayfs.

echo test > /mnt/overlay/testfile
chmod a+s /mnt/overlay/testfile
chown test:test /mnt/overlay/testfile
su test -c "echo > /mnt/overlay/testfile"

The suid bit should be cleard after being written by test user.

Fix it by updating attr on ovl level inode in ovl_setattr(), so the attr is consistent with inode in upper level.

Signed-off-by Eryu Guan <[email protected]>
---
 fs/overlayfs/inode.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
fs/overlayfs/inode.c +11 -0
index 4060ffd..dbe0815 100644
@@ -50,11 +50,22 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr)
 	if (!err) {
 		upperdentry = ovl_dentry_upper(dentry);
 
+		/*
+		 * notify_change() assumes ATTR_KILL_[SG]ID and ATTR_MODE are
+		 * not set at the same time. Clear ATTR_MODE before we call the
+		 * next notify_change().
+		 */
+		if ((attr->ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID)) &&
+		    (attr->ia_valid & ATTR_MODE))
+			attr->ia_valid &= ~ATTR_MODE;
 		mutex_lock(&upperdentry->d_inode->i_mutex);
 		err = notify_change(upperdentry, attr, NULL);
 		mutex_unlock(&upperdentry->d_inode->i_mutex);
 	}
 	ovl_drop_write(dentry);
+	/* Also update the inode in ovl level, for consistency. */
+	if (!err)
+		setattr_copy(d_inode(dentry), attr);
 out:
 	return err;
 }
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH 2/2 ovl: check POSIX ACL support aginst upper layer

Default ACL is not inherited on overlayfs. e.g.

umask 022
mkdir /mnt/overlay/acltestdir
setfacl -d --set u::rwx,g::rwx,o::- /mnt/overlay/acltestdir mkdir /mnt/overlay/acltestdir/subdir
getfacl -p /mnt/overlay/acltestdir/subdir

subdir should inherit the default acl from acltestdir, which is u::rwx,g::rwx,o::-, but subdir has mode specified by umask instead. xfstests generic/314 and generic/319 reveal this issue.

Fix it by setting MS_POSIXACL flag overlayfs superblock to avoid setting mode according to umask, then moving the POSIX ACL handling to overlayfs.

Signed-off-by Eryu Guan <[email protected]>
---
 fs/overlayfs/dir.c   | 22 ++++++++++++++--------
 fs/overlayfs/super.c |  7 +++++++
 2 files changed, 21 insertions(+), 8 deletions(-)
fs/overlayfs/dir.c +14 -8
index 692ceda..8b763c2 100644
@@ -387,20 +387,26 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev,
 {
 	int err;
 	struct inode *inode;
-	struct kstat stat = {
-		.mode = mode,
-		.rdev = rdev,
-	};
+	struct dentry *upperdir;
+	struct kstat stat;
+
+	err = ovl_copy_up(dentry->d_parent);
+	if (err)
+		goto out;
+
+	/* Check POSIX ACL support against upper layer */
+	upperdir = ovl_dentry_upper(dentry->d_parent);
+	if (!IS_POSIXACL(d_inode(upperdir)))
+		mode &= ~current_umask();
+
+	stat.mode = mode;
+	stat.rdev = rdev;
 
 	err = -ENOMEM;
 	inode = ovl_new_inode(dentry->d_sb, mode, dentry->d_fsdata);
 	if (!inode)
 		goto out;
 
-	err = ovl_copy_up(dentry->d_parent);
-	if (err)
-		goto out_iput;
-
 	if (!ovl_dentry_is_opaque(dentry)) {
 		err = ovl_create_upper(dentry, inode, &stat, link, hardlink);
 	} else {
fs/overlayfs/super.c +7 -0
index e38ee0f..1cf923c 100644
@@ -1024,6 +1024,13 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	if (!ufs->upper_mnt)
 		sb->s_flags |= MS_RDONLY;
 
+	/*
+	 * Set MS_POSIXACL unconditionally, so file mode won't be set according
+	 * to umask in vfs layer, POSIX ACL support and new file mode will be
+	 * handled in overlayfs.
+	 */
+	sb->s_flags |= MS_POSIXACL;
+
 	if (remote)
 		sb->s_d_op = &ovl_reval_dentry_operations;
 	else
Signature · 1 line
1.8.3.1