public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
From: akpm@linux-foundation.org
To: viro@zeniv.linux.org.uk
Cc: linux-fsdevel@vger.kernel.org, akpm@linux-foundation.org,
	amwang@redhat.com, eparis@redhat.com, esandeen@redhat.com,
	eteo@redhat.com
Subject: [patch 12/12] vfs: allow file truncations when both suid and write permissions set
Date: Thu, 06 Aug 2009 16:10:18 -0700	[thread overview]
Message-ID: <200908062310.n76NAIEo013014@imap1.linux-foundation.org> (raw)

From: Amerigo Wang <amwang@redhat.com>

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, the 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@redhat.com>
Cc: Eric Sandeen <esandeen@redhat.com>
Cc: Eric Paris <eparis@redhat.com>
Cc: Eugene Teo <eteo@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/open.c          |    8 ++++++--
 include/linux/fs.h |    1 +
 mm/filemap.c       |   12 ++++++++++--
 3 files changed, 17 insertions(+), 4 deletions(-)

diff -puN fs/open.c~vfs-allow-file-truncations-when-both-suid-and-write-permissions-set fs/open.c
--- a/fs/open.c~vfs-allow-file-truncations-when-both-suid-and-write-permissions-set
+++ a/fs/open.c
@@ -213,11 +213,15 @@ int do_truncate(struct dentry *dentry, l
 		newattrs.ia_valid |= ATTR_FILE;
 	}
 
+	mutex_lock(&dentry->d_inode->i_mutex);
 	/* Remove suid/sgid on truncate too */
-	newattrs.ia_valid |= should_remove_suid(dentry);
+	err = dentry_remove_suid(dentry);
+	if (err)
+		goto unlock;
 
-	mutex_lock(&dentry->d_inode->i_mutex);
 	err = notify_change(dentry, &newattrs);
+
+ unlock:
 	mutex_unlock(&dentry->d_inode->i_mutex);
 	return err;
 }
diff -puN include/linux/fs.h~vfs-allow-file-truncations-when-both-suid-and-write-permissions-set include/linux/fs.h
--- a/include/linux/fs.h~vfs-allow-file-truncations-when-both-suid-and-write-permissions-set
+++ a/include/linux/fs.h
@@ -2170,6 +2170,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 -puN mm/filemap.c~vfs-allow-file-truncations-when-both-suid-and-write-permissions-set mm/filemap.c
--- a/mm/filemap.c~vfs-allow-file-truncations-when-both-suid-and-write-permissions-set
+++ a/mm/filemap.c
@@ -1839,9 +1839,11 @@ static int __remove_suid(struct dentry *
 	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,
_

             reply	other threads:[~2009-08-06 23:19 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-06 23:10 akpm [this message]
2009-08-07  2:32 ` [patch 12/12] vfs: allow file truncations when both suid and write permissions set OGAWA Hirofumi
2009-08-07  3:21   ` Amerigo Wang
2009-08-07  4:17     ` OGAWA Hirofumi
2009-08-07  5:49       ` OGAWA Hirofumi
2009-08-07  9:20         ` Amerigo Wang
2009-08-07 11:06           ` OGAWA Hirofumi
2009-08-07  9:27       ` Amerigo Wang
2009-08-07 11:02         ` OGAWA Hirofumi
2009-08-07 11:25           ` OGAWA Hirofumi
2009-08-10  2:00             ` Amerigo Wang
2009-08-10  4:34               ` OGAWA Hirofumi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200908062310.n76NAIEo013014@imap1.linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=amwang@redhat.com \
    --cc=eparis@redhat.com \
    --cc=esandeen@redhat.com \
    --cc=eteo@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox