From: Eric Sandeen <sandeen@sandeen.net> To: Jan Kara <jack@suse.cz>, Al Viro <viro@ZenIV.linux.org.uk> Cc: linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com Subject: Re: [PATCH 5/5] xfs: Correctly lock inode when removing suid and security marks Date: Tue, 14 Apr 2015 11:54:55 -0500 [thread overview] Message-ID: <552D465F.4010700@sandeen.net> (raw) In-Reply-To: <1425379119-3773-6-git-send-email-jack@suse.cz> On 3/3/15 4:38 AM, Jan Kara wrote: > 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. > > Signed-off-by: Jan Kara <jack@suse.cz> This seems like it stands alone, modulo the file_remove_privs function renaming... should it just be pulled into XFS by Dave? I guess that would require the renaming patch to be rebased... Thanks, -Eric > --- > 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 fa81a43702ee..3d601e92bd44 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 > @@ -601,7 +608,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; > } > > /* >
WARNING: multiple messages have this Message-ID (diff)
From: Eric Sandeen <sandeen@sandeen.net> To: Jan Kara <jack@suse.cz>, Al Viro <viro@ZenIV.linux.org.uk> Cc: linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com Subject: Re: [PATCH 5/5] xfs: Correctly lock inode when removing suid and security marks Date: Tue, 14 Apr 2015 11:54:55 -0500 [thread overview] Message-ID: <552D465F.4010700@sandeen.net> (raw) In-Reply-To: <1425379119-3773-6-git-send-email-jack@suse.cz> On 3/3/15 4:38 AM, Jan Kara wrote: > 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. > > Signed-off-by: Jan Kara <jack@suse.cz> This seems like it stands alone, modulo the file_remove_privs function renaming... should it just be pulled into XFS by Dave? I guess that would require the renaming patch to be rebased... Thanks, -Eric > --- > 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 fa81a43702ee..3d601e92bd44 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 > @@ -601,7 +608,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; > } > > /* > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2015-04-14 16:54 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 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 ` Jan Kara 2015-03-03 10:38 ` [PATCH 1/5] fs: Fix S_NOSEC handling Jan Kara 2015-03-03 10:38 ` Jan Kara 2015-03-03 10:38 ` [PATCH 2/5] fs: Rename file_remove_suid() to file_remove_privs() Jan Kara 2015-03-03 10:38 ` Jan Kara 2015-03-03 10:38 ` [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything Jan Kara 2015-03-03 10:38 ` Jan Kara 2015-03-03 10:38 ` [PATCH 4/5] fs: Call security_ops->inode_killpriv on truncate Jan Kara 2015-03-03 10:38 ` 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 [this message] 2015-04-14 16:54 ` Eric Sandeen 2015-04-14 23:03 ` Dave Chinner 2015-04-14 23:03 ` Dave Chinner 2015-05-04 23:13 ` [PATCH 0/5 v2 RESEND] fs: Fixes for removing xid bits and security labels Dave Chinner 2015-05-04 23:13 ` Dave Chinner 2015-05-05 7:56 ` Jan Kara 2015-05-05 7:56 ` Jan Kara -- strict thread matches above, loose matches on Subject: below -- 2015-05-21 14:05 [PATCH 0/5 v4] " Jan Kara 2015-05-21 14:05 ` [PATCH 5/5] xfs: Correctly lock inode when removing suid and security marks Jan Kara 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 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=552D465F.4010700@sandeen.net \ --to=sandeen@sandeen.net \ --cc=jack@suse.cz \ --cc=linux-fsdevel@vger.kernel.org \ --cc=viro@ZenIV.linux.org.uk \ --cc=xfs@oss.sgi.com \ /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: linkBe 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.