On Tue 02-12-14 11:35:48, Brian Foster wrote: > On Tue, Dec 02, 2014 at 04:01:29PM +0100, Jan Kara wrote: > > Currently XFS calls file_remove_suid() 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@xxxxxxx> > > --- > > Hi Jan, > > This doesn't compile... it looks like we need to include the security.h > header. FWIW, even then I get an undefined symbol error when compiling > as a module (security_inode_need_killpriv() does not appear to be > exported). Sorry, forgot to amend the include in the commit. Regarding export of security_inode_need_killpriv() - right, I had security XFS compiled in so I didn't notice. Before I go and fix this up in the obvious way, does anyone have better idea how to fix this than to second guess what file_remove_suid() does? Maybe a VFS helper like file_needs_remove_suid() will be cleaner than what I did? Honza > > fs/xfs/xfs_file.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > > index eb596b419942..ad6636ac4943 100644 > > --- a/fs/xfs/xfs_file.c > > +++ b/fs/xfs/xfs_file.c > > @@ -521,6 +521,18 @@ restart: > > if (error) > > return error; > > > > + /* For changing security info in file_remove_suid() we need i_mutex */ > > + if (!IS_NOSEC(inode) && *iolock == XFS_IOLOCK_SHARED) { > > + struct dentry *dentry = file->f_path.dentry; > > + > > + if (should_remove_suid(dentry) || > > + security_inode_need_killpriv(dentry)) { > > + 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 > > -- > > 1.8.1.4 > > > > _______________________________________________ > > xfs mailing list > > xfs@xxxxxxxxxxx > > http://oss.sgi.com/mailman/listinfo/xfs -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs