Re: [PATCH v2 1/2] xfs: fix tmpfile/selinux deadlock and initialize security/acl

[Brian Foster <bfoster@redhat.com>]

On Thu, Apr 10, 2014 at 03:24:21AM -0700, Christoph Hellwig wrote:
> On Wed, Apr 09, 2014 at 03:21:50PM -0400, Brian Foster wrote:
> > xfs_vn_tmpfile() also fails to initialize security or default acls on
> > the newly created inode.
> 
> Which it doesn't have to, as it is never available in the filesystem
> namespace.
> 

Are you saying it doesn't have to initialize security or the default
acl, or both?

The intent here was to have the case covered where the inode happens to
be linked back into the namespace since we don't do this work in the
link path.

> > The d_tmpfile() call is removed from xfs_create_tmpfile() and pulled up
> > into the new handler to address the deadlock. E.g., xfs_create_tmpfile()
> > has committed the create transaction and unlocked the inode prior to
> > mapping the inode to the dentry.
> 
> This part of the patch looks sane, although the window where the XFS
> inode and VFS inode i_nlink are out of sync worries me a little.
> 
> I don't think the other refactoring belongs into the same patch.
> 
> If we decide that we want it please avoid the useless ACL inheritance
> for tmpfiles.

The bulk of the refactoring was with the idea that the inode setup for
the tmpfile case is generally equivalent for the traditional create
case. The original version was posted here:

http://oss.sgi.com/archives/xfs/2014-04/msg00149.html

... and it just fixes the deadlock and adds the security initialization.
I suppose I could still break that out into multiple patches, but that
aside, is that behavior preferred?

Brian

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs