All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org, dchinner@redhat.com,
	Serge Hallyn <serge.hallyn@canonical.com>,
	linux-security-module@vger.kernel.org, Jan Kara <jack@suse.cz>
Subject: [PATCH 5/5] xfs: Correctly lock inode when removing suid and security marks
Date: Thu, 21 May 2015 16:05:57 +0200	[thread overview]
Message-ID: <1432217157-27876-7-git-send-email-jack@suse.cz> (raw)
In-Reply-To: <1432217157-27876-1-git-send-email-jack@suse.cz>

Currently XFS calls file_remove_privs() without holding i_mutex. This is
wrong because that function can end up messing with file permissions and
security xattrs for which we need i_mutex held.

Fix the problem by grabbing iolock exclusively when we will need to
change anything in permissions / xattrs.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/xfs/xfs_file.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index f3e4fbb59985..71c2c712e609 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -563,6 +563,13 @@ restart:
 	if (error)
 		return error;
 
+	/* For changing security info in file_remove_privs() we need i_mutex */
+	if (*iolock == XFS_IOLOCK_SHARED && !IS_NOSEC(inode)) {
+		xfs_rw_iunlock(ip, *iolock);
+		*iolock = XFS_IOLOCK_EXCL;
+		xfs_rw_ilock(ip, *iolock);
+		goto restart;
+	}
 	/*
 	 * If the offset is beyond the size of the file, we need to zero any
 	 * blocks that fall between the existing EOF and the start of this
@@ -623,7 +630,9 @@ restart:
 	 * setgid bits if the process is not being run by root.  This keeps
 	 * people from modifying setuid and setgid binaries.
 	 */
-	return file_remove_privs(file);
+	if (!IS_NOSEC(inode))
+		return file_remove_privs(file);
+	return 0;
 }
 
 /*
-- 
2.1.4


  parent reply	other threads:[~2015-05-21 14:06 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-21 14:05 [PATCH 0/5 v4] fs: Fixes for removing xid bits and security labels Jan Kara
2015-05-21 14:05 ` [PATCH 1/5] fs: Fix S_NOSEC handling Jan Kara
2015-05-21 14:05 ` [PATCH 2/5] fs: Rename file_remove_suid() to file_remove_privs() Jan Kara
2015-05-21 14:05 ` [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything Jan Kara
2015-05-21 14:05 ` [PATCH 4/5] fs: Call security_ops->inode_killpriv on truncate Jan Kara
2015-05-21 14:05 ` [PATCH 5/5] xfs: Correctly lock inode when removing suid and file capabilities Jan Kara
2015-05-21 14:05 ` Jan Kara [this message]
  -- strict thread matches above, loose matches on Subject: below --
2015-05-19  9:46 [PATCH 0/5 v3] fs: Fixes for removing xid bits and security labels Jan Kara
2015-05-19  9:46 ` [PATCH 5/5] xfs: Correctly lock inode when removing suid and security marks Jan Kara
2015-03-03 10:38 [PATCH 0/5 v2 RESEND] fs: Fixes for removing xid bits and security labels Jan Kara
2015-03-03 10:38 ` [PATCH 5/5] xfs: Correctly lock inode when removing suid and security marks Jan Kara
2015-03-03 10:38   ` Jan Kara
2015-03-03 21:34   ` Dave Chinner
2015-03-03 21:34     ` Dave Chinner
2015-04-14 16:54   ` Eric Sandeen
2015-04-14 16:54     ` Eric Sandeen
2015-04-14 23:03     ` Dave Chinner
2015-04-14 23:03       ` Dave Chinner
2014-12-18 12:49 [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels Jan Kara
2014-12-18 12:49 ` [PATCH 5/5] xfs: Correctly lock inode when removing suid and security marks Jan Kara
2014-12-18 12:49   ` Jan Kara
2014-12-04 13:27 [PATCH 0/5] fs: Fixes for removing xid bits and security labels Jan Kara
2014-12-04 13:27 ` [PATCH 5/5] xfs: Correctly lock inode when removing suid and security marks Jan Kara
2014-12-04 13:27   ` Jan Kara

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=1432217157-27876-7-git-send-email-jack@suse.cz \
    --to=jack@suse.cz \
    --cc=dchinner@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=serge.hallyn@canonical.com \
    --cc=torvalds@linux-foundation.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.