Re: [PATCH] nfs: fix oops when trying to set SELinux label

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2013-11-01 at 13:58 -0400, Jeff Layton wrote:
+AD4- On Fri, 1 Nov 2013 17:47:46 +-0000
+AD4- +ACI-Myklebust, Trond+ACI- +ADw-Trond.Myklebust+AEA-netapp.com+AD4- wrote:
+AD4- 
+AD4- +AD4- On Fri, 2013-11-01 at 13:18 -0400, Jeff Layton wrote:
+AD4- +AD4- +AD4- On Fri, 1 Nov 2013 17:05:08 +-0000
+AD4- +AD4- +AD4- +ACI-Myklebust, Trond+ACI- +ADw-Trond.Myklebust+AEA-netapp.com+AD4- wrote:
+AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- +AD4- On Nov 1, 2013, at 12:57, Jeff Layton +ADw-jlayton+AEA-redhat.com+AD4- wrote:
+AD4- +AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- +AD4- +AD4- On Fri, 1 Nov 2013 16:50:00 +-0000
+AD4- +AD4- +AD4- +AD4- +AD4- +ACI-Myklebust, Trond+ACI- +ADw-Trond.Myklebust+AEA-netapp.com+AD4- wrote:
+AD4- +AD4- +AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- +AD4- +AD4APg- On Fri, 2013-11-01 at 12:02 -0400, Jeff Layton wrote:
+AD4- +AD4- +AD4- +AD4- +AD4APgA+- It looks like +AF8-nfs4+AF8-get+AF8-security+AF8-label() has the same problem, but I've
+AD4- +AD4- +AD4- +AD4- +AD4APgA+- so far been unable to get it to be called, so I didn't patch it. It
+AD4- +AD4- +AD4- +AD4- +AD4APgA+- seems like getxattr does some special stuff for SELinux labels that
+AD4- +AD4- +AD4- +AD4- +AD4APgA+- cause them only to ever be fetched once.
+AD4- +AD4- +AD4- +AD4- +AD4APgA+- 
+AD4- +AD4- +AD4- +AD4- +AD4APgA+- Is there some trick to it?
+AD4- +AD4- +AD4- +AD4- +AD4APgA+- 
+AD4- +AD4- +AD4- +AD4- +AD4APg- 
+AD4- +AD4- +AD4- +AD4- +AD4APg- Doesn't 'ls -Z' cause them to security label to be read again?
+AD4- +AD4- +AD4- +AD4- +AD4APg- 
+AD4- +AD4- +AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- +AD4- +AD4- As best I can tell, security labels are set on the inode when the inode
+AD4- +AD4- +AD4- +AD4- +AD4- is instantiated, and then are reset on changes (i.e. setxattr). If
+AD4- +AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- +AD4- +ICY-and on getxattr, afaics.
+AD4- +AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- I don't see that. The call chain is something like this:
+AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- vfs+AF8-getxattr
+AD4- +AD4- +AD4-   xattr+AF8-getsecurity
+AD4- +AD4- +AD4-     security+AF8-inode+AF8-getsecurity
+AD4- +AD4- +AD4-        selinux+AF8-inode+AF8-getsecurity
+AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- ...and that function looks like it just converts the current security
+AD4- +AD4- +AD4- context on the inode to text and plops that into the buffer.
+AD4- +AD4- 
+AD4- +AD4- Ah, you're right. You have to turn off SELinux in order to hit
+AD4- +AD4- nfs4+AF8-xattr+AF8-get+AF8-nfs4+AF8-label.
+AD4- +AD4- 
+AD4- +AD4- +AD4- +AD4- +AD4- another client changes the label though, it's not clear to me how your
+AD4- +AD4- +AD4- +AD4- +AD4- client would ever notice it until the inode is dropped from the cache.
+AD4- +AD4- +AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- +AD4- +AD4- ISTR Eric Paris explaining to me that they do that for performance
+AD4- +AD4- +AD4- +AD4- +AD4- reasons but it seems like something that needs to be reconsidered in
+AD4- +AD4- +AD4- +AD4- +AD4- light of labeled NFS. Not picking up a security label change seems like
+AD4- +AD4- +AD4- +AD4- +AD4- a bug, IMO...
+AD4- +AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- +AD4- To be effective, the security label should normally be set at file creation time. It should rarely, if ever, change. Why would you need to change it from a different client?
+AD4- +AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- At least in Fedora, there are SELinux policy changes all the time.
+AD4- +AD4- +AD4- Sometimes that involves changing how files are labeled. I don't think
+AD4- +AD4- +AD4- it's reasonable to assume that they only get set at creation time.
+AD4- +AD4- +AD4- 
+AD4- +AD4- 
+AD4- +AD4- That doesn't answer the question. Again, why would you need to do this
+AD4- +AD4- from another client? If you don't have a real-life use case, then it's
+AD4- +AD4- just another 'doctor it hurts' problem. Stop doing it...
+AD4- +AD4- 
+AD4- 
+AD4- Ok, how about this then: +ADs-)
+AD4- 
+AD4- NFS doesn't have O+AF8-TMPFILE, so you really +ACo-can't+ACo- set the context at
+AD4- creation time, at least with normal syscalls. There will always be a
+AD4- race window between creating a file and setting the SELinux context on
+AD4- it.

Ummm. This is supposed to be a security feature. How can you tolerate race windows?

IOW: If I get the security label +ACI-wrong+ACI- on lookup due to such a race,
then how am I supposed to know not to act on it?

+AD4- FWIW, I just started playing with this stuff and this behavior just
+AD4- gave me pause. This is the sort of thing that will give people fits
+AD4- since it's unexpected behavior.
+AD4- 
+AD4- When I change something on the server, I typically expect the client to
+AD4- see that change in a timely fashion. As it stands now, it won't -- at
+AD4- least until the inode gets purged from the cache.

See my previous answer: don't do that... If you want a different answer,
then feel free to propose a caching model, but note that it's going to
be very hard to deal sensibly with your race condition above. That's the
reason why we went with the current caching model.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust+AEA-netapp.com
www.netapp.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux