> > In the "pull" model that GFS2, OCFS2, and GPFS apparently follow, on a
> > given node the file system code doesn't know that an xattr in a
> > security.* namespace got modified on another node. All that is known
> > is that the lock protecting inode xattrs has been lost, so now
> > everything protected by this lock is invalid. The next time this
> > information is needed, it has to be re-initialized (e.g. by reading
> > data from disk). The security attribute label may or may not have
> > changed, that is not known at that point, but the inode security state
> > has to be re-initialized in case it did. So it sounds like all
> > distributed file systems would benefit from an LSM API call that
> > invalidates inode security state.
> >
> > I think the tricky part here is how/when to drive inode security state
> > refresh, once it has been invalidated. File system code could do this
> > easily at dentry revalidation time, possibly even using existing API,
> > like security_d_instantiate (provided it does a full re-init, using
> > getxattr, if it sees that the inode state has been invalidated). A
> > dentry is conveniently available at this juncture.
> > Permission checking prior to a write on an already open file is a
> > somewhat different problem. In the current code, the permission check
> > is made before file system code gets involved, so the call to re-init
> > inode security state has to be made in VFS or LSM code, before the
> > permission check. It would probably be easier/more logical to do this
> > in security_file_permission. Since the latter is passed struct file
> > *file, a dentry could be easily obtained as file->f_path.dentry.
>
> The two problems I see with doing the refresh 'when needed' is that FS
> code is almost certainly not designed to have a ->getxattr call during a
> read/write and not all permissions checks have a dentry. Namely any
> calls to fs/namei.c::inode_permission() are going to end up in the LSM
> without a dentry and with an 'invalid' security struct. I feel like the
> invalidate and refresh need to be done at the same time....
The invalidate and refresh can't really be done at the same time. The invalidate would happen at a point where a node has to relinquish a distributed lock. If in the process of relinquishing a lock one tried to synchronously run an operation that requires the same lock (getxattr), that would deadlock. So the refresh has to come at a later point.
I think the scope of the "when to revalidate" problem here is not as large as it may seem. Cluster file systems already have the same invalidation/revalidation problem with the struct inode content. In most code paths, a valid dentry is needed to get things going, and if the dentry has been marked as needing revalidation, it'll be revalidated before the first use. The specific case of security_file_permission() being called from rw_verify_area() would be an exception, since on vanilla Unix no permission checking would be done prior to a read/write on an already open file. I don't think a getxattr() call from inside rw_verify_area() would be a problem for file system code, since the file system-specific code hasn't been entered yet. It would be somewhat difficult to verify that all special cases of this nature are covered, however.
It is not that hard to look up _a_ dentry given an inode pointer, one just has to look at the inode's i_dentry list. The problem is that multiple dentry's could be pointing to the same inode (e.g. for a regular file with multiple hardlinks). I don't know whether it matters which of the aliases is given as an argument to getxattr. GPFS getxattr iop uses the dentry argument to get to an inode pointer, and doesn't use it for anything else, so any alias dentry would work as good as any other. If this is also the case for other cluster file systems, perhaps doing an inode security state refresh with nothing by inode pointer is not a problem. From the GPFS point of view, as long as the getxattr call is done outside of file system call proper, it shouldn't be a problem. Calling it from inside of another iop/fop could be problematic, depending on what locks are held at this point. There's already precedent for this (security_d_instantiate called from d_splice_alias et al), so with care this can be made to work.
yuri
