linux-kernel - [Patch v3] vfs: allow file truncations when both suid and write permissions set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 7 Aug 2009 06:05:22 -0400
From:	Amerigo Wang <amwang@...hat.com>
To:	linux-kernel@...r.kernel.org
Cc:	esandeen@...hat.com, eteo@...hat.com, eparis@...hat.com,
	Amerigo Wang <amwang@...hat.com>,
	linux-fsdevel@...r.kernel.org, akpm@...ux-foundation.org,
	hirofumi@...l.parknet.co.jp, viro@...iv.linux.org.uk
Subject: [Patch v3] vfs: allow file truncations when both suid and write permissions set


V2 -> V3:
Call notify_change() before clearing suid/sgid.
Thanks to OGAWA Hirofumi.

V1 -> V2:
Introduce dentry_remove_suid(), and use it in do_truncate().
Thanks to Eric Paris.


When suid is set and the non-owner user has write permission,
any writing into this file should be allowed and suid should be
removed after that.

However, current kernel only allows writing without truncations,
when we do truncations on that file, we get EPERM. This is a bug.

Steps to reproduce this bug:

% ls -l rootdir/file1
-rwsrwsrwx 1 root root 3 Jun 25 15:42 rootdir/file1
% echo h > rootdir/file1
zsh: operation not permitted: rootdir/file1
% ls -l rootdir/file1
-rwsrwsrwx 1 root root 3 Jun 25 15:42 rootdir/file1
% echo h >> rootdir/file1
% ls -l rootdir/file1
-rwxrwxrwx 1 root root 5 Jun 25 16:34 rootdir/file1

This patch fixes it.

Signed-off-by: WANG Cong <amwang@...hat.com>
Cc: Eric Sandeen <esandeen@...hat.com>
Cc: Eric Paris <eparis@...hat.com>
Cc: Eugene Teo <eteo@...hat.com>
Cc: Al Viro <viro@...iv.linux.org.uk>
Cc: hirofumi@...l.parknet.co.jp

---
diff --git a/fs/open.c b/fs/open.c
index dd98e80..159f5b3 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -213,11 +213,15 @@ int do_truncate(struct dentry *dentry, loff_t length, unsigned int time_attrs,
 		newattrs.ia_valid |= ATTR_FILE;
 	}
 
-	/* Remove suid/sgid on truncate too */
-	newattrs.ia_valid |= should_remove_suid(dentry);
-
 	mutex_lock(&dentry->d_inode->i_mutex);
 	err = notify_change(dentry, &newattrs);
+	if (err)
+		goto unlock;
+
+	/* Remove suid/sgid on truncate too */
+	err = dentry_remove_suid(dentry);
+
+ unlock:
 	mutex_unlock(&dentry->d_inode->i_mutex);
 	return err;
 }
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a36ffa5..0481f11 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2167,6 +2167,7 @@ extern void destroy_inode(struct inode *);
 extern struct inode *new_inode(struct super_block *);
 extern int should_remove_suid(struct dentry *);
 extern int file_remove_suid(struct file *);
+extern int dentry_remove_suid(struct dentry *);
 
 extern void __insert_inode_hash(struct inode *, unsigned long hashval);
 extern void remove_inode_hash(struct inode *);
diff --git a/mm/filemap.c b/mm/filemap.c
index ccea3b6..33d94ad 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1839,9 +1839,11 @@ static int __remove_suid(struct dentry *dentry, int kill)
 	return notify_change(dentry, &newattrs);
 }
 
-int file_remove_suid(struct file *file)
+/*
+ * Note: you need to hold i_mutex before call this.
+ */
+int dentry_remove_suid(struct dentry *dentry)
 {
-	struct dentry *dentry = file->f_path.dentry;
 	int killsuid = should_remove_suid(dentry);
 	int killpriv = security_inode_need_killpriv(dentry);
 	int error = 0;
@@ -1855,6 +1857,12 @@ int file_remove_suid(struct file *file)
 
 	return error;
 }
+
+int file_remove_suid(struct file *file)
+{
+	struct dentry *dentry = file->f_path.dentry;
+	return dentry_remove_suid(dentry);
+}
 EXPORT_SYMBOL(file_remove_suid);
 
 static size_t __iovec_copy_from_user_inatomic(char *vaddr,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/