2014-04-15 17:52:28

by Christoph Hellwig

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

So any opinions from other fs / security people on how O_TMPFILE files
should behave for ACL inheritance / labeling?

On Thu, Apr 10, 2014 at 05:29:44AM -0700, Christoph Hellwig wrote:
> On Thu, Apr 10, 2014 at 08:19:48AM -0400, Brian Foster wrote:
> > Are you saying it doesn't have to initialize security or the default
> > acl, or both?
>
> The ACLs for sure. LSM do run-time access decisions, so they will
> probably rely on the security data being initialized. Given that
> O_TMPFILE files aren't publicly available I'm not sure there's a point
> in them doing that, though.
>
> LSMs are also affected by the lack of a proper parent I'll discuss for
> ACLs below.
>
> > 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.
>
> That's an interesting one. O_TMFILE files don't have a real parent
> to inherit ACLs from, the pathname passed in just needs to point to
> a directory to find the filesystem to create the tmpfile in. On
> the other hand it seem like the extN implementations do inherity the
> ACL in this case.
>
> The link into the namespace is irrelavant here as ACL inheritance only
> happens on initial create, not at link time.
>
> I also think we'll absolutely need a test case for ACLs+tmpfile to
> make sure all filesystems handle it the same way.
>
> > 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?
>
> I think just fixing the deadlock and initializing the security is enough
> for the first pass. If you want to do the refactoring on top send it as
> a second series on top of the actual fixes.
>
> _______________________________________________
> xfs mailing list
> [email protected]
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---


2014-04-15 19:31:02

by Andreas Gruenbacher

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

Christoph,

On Tue, Apr 15, 2014 at 10:52:28AM -0700, Christoph Hellwig wrote:
> So any opinions from other fs / security people on how O_TMPFILE files
> should behave for ACL inheritance / labeling?

from how O_TMPFILE is documented right now [*], creating such a file and
then linking it into the namespace is one of the obvious use cases. The
intent seems to be to make it seem like the file was created and populated
atomically, possibly with inherited permissions and all. I think this
behavior require that the O_TMPFILE file inherits from the directory it
was "created" in.

Adding code to achieve the effect of create-time inheritance at link
time, only for O_TMPFILE files or files without any links, doesn't seem
reasonable to me: it would duplicate create code in the link code path,
and it would make it harder to override inherited permissions or labels.

(Trying to fake inheritance by reimplementing it in user space seems like
a much worse idea still.)

[*] http://man7.org/linux/man-pages/man2/open.2.html

Thanks,
Andreas

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs