diff mbox

ovl: fix sgid inhertance over whiteout

Message ID 1472112902-11767-1-git-send-email-guaneryu@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eryu Guan Aug. 25, 2016, 8:15 a.m. UTC
In commit bb0d2b8ad296 ("ovl: fix sgid on directory"), mode of newly
created files & dirs over whiteout was updated to pickup sgid bit. But
it used stat->mode directly, which could not be the correct mode,
because stat->mode could be tailored against umask.

This can be demonstrated by the following script:

umask 022
mkdir -p lower/dir upper work mnt
chown test:test lower/dir
chmod 2775 lower/dir
touch lower/dir/testdir
touch lower/dir/testfile

mount -t overlay -olowerdir=lower,upperdir=upper,workdir=work none mnt
rm -f mnt/dir/testdir
mkdir mnt/dir/testdir
rm -f mnt/dir/testfile
touch mnt/dir/testfile
touch mnt/dir/newfile
ls -l mnt/dir

-rw-r--r--. 1 root test 0 Aug 25 15:45 newfile
drwxrwsrwx. 2 root test 6 Aug 25 15:45 testdir
-rw-rw-rw-. 1 root test 0 Aug 25 15:45 testfile

testdir and testfile are created over whiteout, the modes contain write
permission for group and other, but they shouldn't, like 'newfile'.

Fix it by resetting mode against upperdir inode using
inode_init_owner().

Fixes: bb0d2b8ad296 ("ovl: fix sgid on directory")
Signed-off-by: Eryu Guan <guaneryu@gmail.com>
---

A new fstests test case followed to fstests list.

 fs/overlayfs/dir.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
diff mbox

Patch

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 12bcd07..9a5e716 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -370,17 +370,19 @@  static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
 		goto out_dput2;
 
 	/*
-	 * mode could have been mutilated due to umask (e.g. sgid directory)
+	 * mode could lose sgid bit if upperdir has it set, because workdir has
+	 * no sgid. Reset mode against upperdir.
 	 */
-	if (!hardlink &&
-	    !S_ISLNK(stat->mode) && newdentry->d_inode->i_mode != stat->mode) {
+	if (!hardlink && !S_ISLNK(stat->mode)) {
+		struct inode *newinode = newdentry->d_inode;
 		struct iattr attr = {
 			.ia_valid = ATTR_MODE,
-			.ia_mode = stat->mode,
 		};
-		inode_lock(newdentry->d_inode);
+		inode_init_owner(newinode, udir, newinode->i_mode);
+		attr.ia_mode = newinode->i_mode;
+		inode_lock(newinode);
 		err = notify_change(newdentry, &attr, NULL);
-		inode_unlock(newdentry->d_inode);
+		inode_unlock(newinode);
 		if (err)
 			goto out_cleanup;
 	}